Skip to content

BrowserBookmark do not use SqliteConnection pooling to avoid ObjectDisposedException #3674

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

Merged
merged 3 commits into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@
var profileBookmarks = LoadBookmarksFromFile(bookmarkPath, source);

// Load favicons after loading bookmarks
if (Main._settings.EnableFavicons)

Check warning on line 55 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
{
var faviconDbPath = Path.Combine(profile, "Favicons");

Check warning on line 57 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
if (File.Exists(faviconDbPath))
{
Main._context.API.StopwatchLogInfo(ClassName, $"Load {profileBookmarks.Count} favicons cost", () =>

Check warning on line 60 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`favicons` is not a recognized word. (unrecognized-spelling)
{
LoadFaviconsFromDb(faviconDbPath, profileBookmarks);

Check warning on line 62 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
});
}
}
Expand Down Expand Up @@ -129,7 +129,7 @@
}
}

private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)

Check warning on line 132 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`Favicons` is not a recognized word. (unrecognized-spelling)
{
// Use a copy to avoid lock issues with the original file
var tempDbPath = Path.Combine(_faviconCacheDir, $"tempfavicons_{Guid.NewGuid()}.db");
Expand All @@ -155,12 +155,15 @@
return;
}

// Cache connection for pool clean
SqliteConnection connection1 = null;

try
{
// Since some bookmarks may have same favicon id, we need to record them to avoid duplicates
var savedPaths = new ConcurrentDictionary<string, bool>();

// Get favicons based on bookmarks concurrently

Check warning on line 166 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`favicons` is not a recognized word. (unrecognized-spelling)
Parallel.ForEach(bookmarks, bookmark =>
{
// Use read-only connection to avoid locking issues
Expand All @@ -181,7 +184,7 @@
using var cmd = connection.CreateCommand();
cmd.CommandText = @"
SELECT f.id, b.image_data
FROM favicons f

Check warning on line 187 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`favicons` is not a recognized word. (unrecognized-spelling)
JOIN favicon_bitmaps b ON f.id = b.icon_id
JOIN icon_mapping m ON f.id = m.icon_id
WHERE m.page_url LIKE @url
Expand All @@ -202,7 +205,7 @@

var faviconPath = Path.Combine(_faviconCacheDir, $"chromium_{domain}_{iconId}.png");

// Filter out duplicate favicons

Check warning on line 208 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`favicons` is not a recognized word. (unrecognized-spelling)
if (savedPaths.TryAdd(faviconPath, true))
{
SaveBitmapData(imageData, faviconPath);
Expand All @@ -216,8 +219,9 @@
}
finally
{
// https://github.com/dotnet/efcore/issues/26580
SqliteConnection.ClearPool(connection);
// Cache connection and clear pool after all operations to avoid issue:
// ObjectDisposedException: Safe handle has been closed.
connection1 = connection;
connection.Close();
connection.Dispose();
}
Expand All @@ -231,6 +235,11 @@
// Delete temporary file
try
{
// https://github.com/dotnet/efcore/issues/26580
if (connection1 != null)
{
SqliteConnection.ClearPool(connection1);
}
File.Delete(tempDbPath);
}
catch (Exception ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@

// Updated query - removed favicon_id column
private const string QueryAllBookmarks = """
SELECT moz_places.url, moz_bookmarks.title

Check warning on line 27 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/FirefoxBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`moz` is not a recognized word. (unrecognized-spelling)
FROM moz_places

Check warning on line 28 in Plugins/Flow.Launcher.Plugin.BrowserBookmark/FirefoxBookmarkLoader.cs

View workflow job for this annotation

GitHub Actions / Check Spelling

`moz` is not a recognized word. (unrecognized-spelling)
INNER JOIN moz_bookmarks ON (
moz_bookmarks.fk NOT NULL AND moz_bookmarks.title NOT NULL AND moz_bookmarks.fk = moz_places.id
)
Expand Down Expand Up @@ -142,6 +142,9 @@
return;
}

// Cache connection for pool clean
SqliteConnection connection1 = null;

try
{
// Since some bookmarks may have same favicon id, we need to record them to avoid duplicates
Expand Down Expand Up @@ -212,8 +215,9 @@
}
finally
{
// https://github.com/dotnet/efcore/issues/26580
SqliteConnection.ClearPool(connection);
// Cache connection and clear pool after all operations to avoid issue:
// ObjectDisposedException: Safe handle has been closed.
connection1 = connection;
connection.Close();
connection.Dispose();
}
Expand All @@ -227,6 +231,11 @@
// Delete temporary file
try
{
// https://github.com/dotnet/efcore/issues/26580
if (connection1 != null)
{
SqliteConnection.ClearPool(connection1);
}
File.Delete(tempDbPath);
}
catch (Exception ex)
Expand Down
Loading