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.

Sign up for GitHub

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
Jack251970 merged 3 commits into from
Jun 13, 2025
Merged

BrowserBookmark do not use SqliteConnection pooling to avoid ObjectDisposedException #3674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c8e82cb
Cache connection and clear pool after all operations to avoid ObjectD…
Jack251970 Jun 7, 2025
2ed32b3
Do not use pooling so that we do not need to clear pool
Jack251970 Jun 8, 2025
d7b8f85
Use FaviconHelper
Jack251970 Jun 8, 2025
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
78 changes: 15 additions & 63 deletions Plugins/Flow.Launcher.Plugin.BrowserBookmark/ChromiumBookmarkLoader.cs
View file
Open in desktop
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 @@ -131,31 +132,7 @@ private static void EnumerateFolderBookmark(JsonElement folderElement, ICollecti

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()}.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>();
Expand All @@ -164,7 +141,8 @@ private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)
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 @@ private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)

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
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 @@ -205,7 +183,7 @@ ORDER BY b.width DESC
// Filter out duplicate favicons
if (savedPaths.TryAdd(faviconPath, true))
{
SaveBitmapData(imageData, faviconPath);
FaviconHelper.SaveBitmapData(imageData, faviconPath);
}

bookmark.FaviconPath = faviconPath;
Expand All @@ -216,38 +194,12 @@ ORDER BY b.width DESC
}
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);
}
});
}
}
75 changes: 9 additions & 66 deletions Plugins/Flow.Launcher.Plugin.BrowserBookmark/FirefoxBookmarkLoader.cs
View file
Open in desktop
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 Down Expand Up @@ -118,31 +119,7 @@ protected List<Bookmark> GetBookmarksFromPath(string placesPath)

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 @@ private void LoadFaviconsFromDb(string dbPath, List<Bookmark> bookmarks)
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 @@ ORDER BY i.width DESC -- Select largest icon available
return;

string faviconPath;
if (IsSvgData(imageData))
if (FaviconHelper.IsSvgData(imageData))
{
faviconPath = Path.Combine(_faviconCacheDir, $"firefox_{domain}.svg");
}
Expand All @@ -201,7 +179,7 @@ ORDER BY i.width DESC -- Select largest icon available
// 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 @@ ORDER BY i.width DESC -- Select largest icon available
}
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
76 changes: 76 additions & 0 deletions Plugins/Flow.Launcher.Plugin.BrowserBookmark/Helper/FaviconHelper.cs
View file
Open in desktop
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"));
}
}