Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing tenant with SQLite DB fails due to the DB file being locked #16849

Closed
Piedone opened this issue Oct 8, 2024 · 8 comments · Fixed by #16858
Closed

Removing tenant with SQLite DB fails due to the DB file being locked #16849

Piedone opened this issue Oct 8, 2024 · 8 comments · Fixed by #16858
Assignees
Milestone

Comments

@Piedone
Copy link
Member

Piedone commented Oct 8, 2024

Describe the bug

When trying to delete a tenant that uses SQLite, you'll get an error along the lines of:

An error occurred while removing the tenant 'Demo'. Failed to remove the tables. System.IO.IOException: The process cannot access the file 'E:\Projects\OrchardForks\OrchardCore\src\OrchardCore.Cms.Web\App_Data\Sites\Demo\OrchardCore.db' because it is being used by another process.

Orchard Core version

2.0.1 but reproduces with 8de0e71 too.

This used to work with 1.8.x, so it's a regression

To Reproduce

  1. Create and set up a new tenant.
  2. Disable it. Don't restart the app.
  3. Try to remove it from the tenants admin or via the API (TenantRemovalAllowed is needed for this).
  4. Observe the error. Restarting the app fixes the issue.

I suppose the tenant is not actually disabled and keeps running, or at least some part of it.

Expected behavior

The tenant is removed without issues.

Logs and screenshots

2024-10-08_22h14_35.mp4
2024-10-08 19:53:43.4063|Default|00-00bea62d3977adf61a2bb544d30b413b-ffb248a785a46cbc-00||OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler|ERROR|Failed to remove the tables of tenant 'UITestTenant'. System.IO.IOException: The process cannot access the file 'D:\a\Open-Source-Orchard-Core-Extensions\Open-Source-Orchard-Core-Extensions\NuGetTest\test\Lombiq.OSOCE.NuGet.Tests.UI\bin\Debug\net8.0\Temp\27d5e5da-44cb-4146-b967-62579a5f0a79\App\App_Data\Sites\UITestTenant\OrchardCore.db' because it is being used by another process.
   at System.IO.FileSystem.DeleteFile(String fullPath)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)    at System.IO.FileSystem.DeleteFile(String fullPath)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)
   at OrchardCore.Environment.Shell.Removing.ShellDbTablesRemovingHandler.RemovingAsync(ShellRemovingContext context)
@MikeAlhayek
Copy link
Member

MikeAlhayek commented Oct 8, 2024

If you want to try resolving the issue, maybe try running the logic in ShellDbTablesRemovingHandler.RemovingAsync in a background task. specially these lines

SqliteConnection.ClearPool(sqliteConnection);
File.Delete(connection.DataSource);

or dispose connection before line 81.

@Piedone
Copy link
Member Author

Piedone commented Oct 9, 2024

I noticed that this only happens after a fresh setup. So, if you restart the app after a setup, then even if you open the tenant and disable it after that, the removal will succeed.

So, I don't think a bg task would help. Somehow the setup is locking the file.

@MikeAlhayek
Copy link
Member

If you run it as a background task, the task will run at the end of the request. This should be executed after the file is released. Worth trying

@Piedone
Copy link
Member Author

Piedone commented Oct 9, 2024

The DB file remains locked even after many requests afterward. So, something is locking it across requests (but that's released and not locked again when the whole app is restarted).

@MikeAlhayek
Copy link
Member

MikeAlhayek commented Oct 9, 2024

Try disabling pooling. Please try adding this code and see if this will fix the issue

services.Configure<SqliteOptions>(options => 
{
    options.UseConnectionPooling = false;
});

more info

/// <summary>
/// Sqlite-specific configuration for the Orchard Core database. 
/// See <see href="https://docs.orchardcore.net/en/latest/reference/core/Data/#sqlite" />.
/// </summary>
public class SqliteOptions
{
    /// <summary>
    /// <para>By default in .Net 6, <c>Microsoft.Data.Sqlite</c> pools connections to the database. 
    /// It achieves this by putting a lock on the database file and leaving connections open to be reused.
    /// If the lock is preventing tasks like backups, this functionality can be disabled.</para>
    /// 
    /// <para>There may be a performance penalty associated with disabling connection pooling.</para>
    /// <see href="https://docs.microsoft.com/en-us/dotnet/standard/data/sqlite/connection-strings#pooling" />.
    /// </summary>
    public bool UseConnectionPooling { get; set; } = true;
}

@Piedone
Copy link
Member Author

Piedone commented Oct 9, 2024

Yeah, thanks, that helps, as does SqliteConnection.ClearAllPools(); instead of SqliteConnection.ClearPool(sqliteConnection);. I'm trying to figure out what keeps a connection alive, though: This hints on there being multiple SQLite connection strings, and thus multiple connection pools, for the same DB file.

@Piedone
Copy link
Member Author

Piedone commented Oct 9, 2024

Hmm yeah, for the same tenant/DB there are three pools:

  • OrchardCore.db;Mode=ReadWriteCreate: this is the one created for the setup, when the tenant is initializing. It should be released after setup, I'll try to fix this.
  • OrchardCore.db;Mode=ReadWrite: this is the correct one.
  • yessql.db;Mode=ReadWrite: I don't know where this comes from, but apparently, somewhere the DatabaseName config is not being used. This is all for a brand new tenant though, so it's not like it should be there for backward compatibility.

Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants