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 all commits
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 @@ -4,6 +4,7 @@
using System.IO;
using System.Text.Json;
using System.Threading.Tasks;
using Flow.Launcher.Plugin.BrowserBookmark.Helper;
using Flow.Launcher.Plugin.BrowserBookmark.Models;
using Microsoft.Data.Sqlite;

Expand Down Expand Up @@ -52,14 +53,14 @@
var profileBookmarks = LoadBookmarksFromFile(bookmarkPath, source);

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

Check warning on line 56 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 58 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 61 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 63 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,42 +130,19 @@
}
}

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

Check warning on line 133 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");

try
{
File.Copy(dbPath, tempDbPath, true);
}
catch (Exception ex)
{
try
{
if (File.Exists(tempDbPath))
{
File.Delete(tempDbPath);
}
}
catch (Exception ex1)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex1);
}
Main._context.API.LogException(ClassName, $"Failed to copy favicon DB: {dbPath}", ex);
return;
}

try
FaviconHelper.LoadFaviconsFromDb(_faviconCacheDir, dbPath, (tempDbPath) =>
{
// 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 140 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
var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly");
// Do not use pooling so that we do not need to clear pool: https://github.com/dotnet/efcore/issues/26580
var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly;Pooling=false");
connection.Open();

try
Expand All @@ -180,13 +158,13 @@

using var cmd = connection.CreateCommand();
cmd.CommandText = @"
SELECT f.id, b.image_data
FROM favicons f
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
ORDER BY b.width DESC
LIMIT 1";
SELECT f.id, b.image_data
FROM favicons f

Check warning on line 162 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
ORDER BY b.width DESC
LIMIT 1";

cmd.Parameters.AddWithValue("@url", $"%{domain}%");

Expand All @@ -202,10 +180,10 @@

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

// Filter out duplicate favicons

Check warning on line 183 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);
FaviconHelper.SaveBitmapData(imageData, faviconPath);
}

bookmark.FaviconPath = faviconPath;
Expand All @@ -216,38 +194,12 @@
}
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.
connection.Close();
connection.Dispose();
}
});
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to connect to SQLite: {tempDbPath}", ex);
}

// Delete temporary file
try
{
File.Delete(tempDbPath);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex);
}
}

private static void SaveBitmapData(byte[] imageData, string outputPath)
{
try
{
File.WriteAllBytes(outputPath, imageData);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to save image: {outputPath}", ex);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Flow.Launcher.Plugin.BrowserBookmark.Helper;
using Flow.Launcher.Plugin.BrowserBookmark.Models;
using Microsoft.Data.Sqlite;

Expand All @@ -24,8 +25,8 @@

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

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)
FROM moz_places

Check warning on line 29 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 @@ -118,31 +119,7 @@

private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)
{
// Use a copy to avoid lock issues with the original file
var tempDbPath = Path.Combine(_faviconCacheDir, $"tempfavicons_{Guid.NewGuid()}.sqlite");

try
{
File.Copy(dbPath, tempDbPath, true);
}
catch (Exception ex)
{
try
{
if (File.Exists(tempDbPath))
{
File.Delete(tempDbPath);
}
}
catch (Exception ex1)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex1);
}
Main._context.API.LogException(ClassName, $"Failed to copy favicon DB: {dbPath}", ex);
return;
}

try
FaviconHelper.LoadFaviconsFromDb(_faviconCacheDir, dbPath, (tempDbPath) =>
{
// Since some bookmarks may have same favicon id, we need to record them to avoid duplicates
var savedPaths = new ConcurrentDictionary<string, bool>();
Expand All @@ -151,7 +128,8 @@
Parallel.ForEach(bookmarks, bookmark =>
{
// Use read-only connection to avoid locking issues
var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly");
// Do not use pooling so that we do not need to clear pool: https://github.com/dotnet/efcore/issues/26580
var connection = new SqliteConnection($"Data Source={tempDbPath};Mode=ReadOnly;Pooling=false");
connection.Open();

try
Expand Down Expand Up @@ -189,7 +167,7 @@
return;

string faviconPath;
if (IsSvgData(imageData))
if (FaviconHelper.IsSvgData(imageData))
{
faviconPath = Path.Combine(_faviconCacheDir, $"firefox_{domain}.svg");
}
Expand All @@ -201,7 +179,7 @@
// Filter out duplicate favicons
if (savedPaths.TryAdd(faviconPath, true))
{
SaveBitmapData(imageData, faviconPath);
FaviconHelper.SaveBitmapData(imageData, faviconPath);
}

bookmark.FaviconPath = faviconPath;
Expand All @@ -212,48 +190,13 @@
}
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.
connection.Close();
connection.Dispose();
}
});
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to load Firefox favicon DB: {tempDbPath}", ex);
}

// Delete temporary file
try
{
File.Delete(tempDbPath);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex);
}
}

private static void SaveBitmapData(byte[] imageData, string outputPath)
{
try
{
File.WriteAllBytes(outputPath, imageData);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to save image: {outputPath}", ex);
}
}

private static bool IsSvgData(byte[] data)
{
if (data.Length < 5)
return false;
string start = System.Text.Encoding.ASCII.GetString(data, 0, Math.Min(100, data.Length));
return start.Contains("<svg") ||
(start.StartsWith("<?xml") && start.Contains("<svg"));
});
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
using System;
using System.IO;

namespace Flow.Launcher.Plugin.BrowserBookmark.Helper;

public static class FaviconHelper
{
private static readonly string ClassName = nameof(FaviconHelper);

public static void LoadFaviconsFromDb(string faviconCacheDir, string dbPath, Action<string> loadAction)
{
// Use a copy to avoid lock issues with the original file
var tempDbPath = Path.Combine(faviconCacheDir, $"tempfavicons_{Guid.NewGuid()}.db");

try
{
File.Copy(dbPath, tempDbPath, true);
}
catch (Exception ex)
{
try
{
if (File.Exists(tempDbPath))
{
File.Delete(tempDbPath);
}
}
catch (Exception ex1)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex1);
}
Main._context.API.LogException(ClassName, $"Failed to copy favicon DB: {dbPath}", ex);
return;
}

try
{
loadAction(tempDbPath);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to connect to SQLite: {tempDbPath}", ex);
}

// Delete temporary file
try
{
File.Delete(tempDbPath);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to delete temporary favicon DB: {tempDbPath}", ex);
}
}

public static void SaveBitmapData(byte[] imageData, string outputPath)
{
try
{
File.WriteAllBytes(outputPath, imageData);
}
catch (Exception ex)
{
Main._context.API.LogException(ClassName, $"Failed to save image: {outputPath}", ex);
}
}

public static bool IsSvgData(byte[] data)
{
if (data.Length < 5)
return false;
string start = System.Text.Encoding.ASCII.GetString(data, 0, Math.Min(100, data.Length));
return start.Contains("<svg") ||
(start.StartsWith("<?xml") && start.Contains("<svg"));
}
}
Loading