From 006f73af7d6af3c077eab926e6ef2e0e542e4803 Mon Sep 17 00:00:00 2001 From: timof <54764164+timofej673@users.noreply.github.com> Date: Mon, 21 Jul 2025 12:02:06 +0400 Subject: [PATCH 1/5] Update message_cache.go Added lock in add_messages to avoid "database is locked" error Small code reformatting --- server/message_cache.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/server/message_cache.go b/server/message_cache.go index e314ace3..a73650ad 100644 --- a/server/message_cache.go +++ b/server/message_cache.go @@ -8,6 +8,7 @@ import ( "net/netip" "strings" "time" + "sync" _ "github.com/mattn/go-sqlite3" // SQLite driver "heckel.io/ntfy/v2/log" @@ -35,7 +36,7 @@ const ( priority INT NOT NULL, tags TEXT NOT NULL, click TEXT NOT NULL, - icon TEXT NOT NULL, + icon TEXT NOT NULL, actions TEXT NOT NULL, attachment_name TEXT NOT NULL, attachment_type TEXT NOT NULL, @@ -72,30 +73,30 @@ const ( selectRowIDFromMessageID = `SELECT id FROM messages WHERE mid = ?` // Do not include topic, see #336 and TestServer_PollSinceID_MultipleTopics selectMessagesByIDQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE mid = ? ` selectMessagesSinceTimeQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE topic = ? AND time >= ? AND published = 1 ORDER BY time, id ` selectMessagesSinceTimeIncludeScheduledQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE topic = ? AND time >= ? ORDER BY time, id ` selectMessagesSinceIDQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE topic = ? AND id > ? AND published = 1 ORDER BY time, id ` selectMessagesSinceIDIncludeScheduledQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE topic = ? AND (id > ? OR published = 0) ORDER BY time, id ` @@ -105,10 +106,10 @@ const ( WHERE topic = ? AND published = 1 ORDER BY time DESC, id DESC LIMIT 1 - ` + ` selectMessagesDueQuery = ` SELECT mid, time, expires, topic, message, title, priority, tags, click, icon, actions, attachment_name, attachment_type, attachment_size, attachment_expires, attachment_url, sender, user, content_type, encoding - FROM messages + FROM messages WHERE time <= ? AND published = 0 ORDER BY time, id ` @@ -281,6 +282,7 @@ var ( type messageCache struct { db *sql.DB queue *util.BatchingQueue[*message] + mu sync.Mutex nop bool } @@ -340,6 +342,8 @@ func (c *messageCache) AddMessage(m *message) error { // addMessages synchronously stores a match of messages. If the database is locked, the transaction waits until // SQLite's busy_timeout is exceeded before erroring out. func (c *messageCache) addMessages(ms []*message) error { + c.mu.Lock() + defer c.mu.Unlock() if c.nop { return nil } From f8082d94811a517bcfc3abf97e5b9cdda573bbf5 Mon Sep 17 00:00:00 2001 From: timof <54764164+timofej673@users.noreply.github.com> Date: Wed, 30 Jul 2025 00:12:45 +0400 Subject: [PATCH 2/5] Update message_cache.go --- server/message_cache.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/message_cache.go b/server/message_cache.go index a73650ad..1ab77b81 100644 --- a/server/message_cache.go +++ b/server/message_cache.go @@ -525,6 +525,8 @@ func (c *messageCache) Message(id string) (*message, error) { } func (c *messageCache) MarkPublished(m *message) error { + c.mu.Lock() + defer c.mu.Unlock() _, err := c.db.Exec(updateMessagePublishedQuery, m.ID) return err } @@ -570,6 +572,8 @@ func (c *messageCache) Topics() (map[string]*topic, error) { } func (c *messageCache) DeleteMessages(ids ...string) error { + c.mu.Lock() + defer c.mu.Unlock() tx, err := c.db.Begin() if err != nil { return err @@ -584,6 +588,8 @@ func (c *messageCache) DeleteMessages(ids ...string) error { } func (c *messageCache) ExpireMessages(topics ...string) error { + c.mu.Lock() + defer c.mu.Unlock() tx, err := c.db.Begin() if err != nil { return err @@ -618,6 +624,8 @@ func (c *messageCache) AttachmentsExpired() ([]string, error) { } func (c *messageCache) MarkAttachmentsDeleted(ids ...string) error { + c.mu.Lock() + defer c.mu.Unlock() tx, err := c.db.Begin() if err != nil { return err @@ -763,6 +771,8 @@ func readMessage(rows *sql.Rows) (*message, error) { } func (c *messageCache) UpdateStats(messages int64) error { + c.mu.Lock() + defer c.mu.Unlock() _, err := c.db.Exec(updateStatsQuery, messages) return err } From fe5c844a21028289be33a750eb8c6e0c1a46c18d Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 8 Aug 2025 16:10:49 -0400 Subject: [PATCH 3/5] Add test --- server/message_cache.go | 4 ++-- server/message_cache_test.go | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/server/message_cache.go b/server/message_cache.go index 64561d02..902cac1c 100644 --- a/server/message_cache.go +++ b/server/message_cache.go @@ -8,8 +8,8 @@ import ( "net/netip" "path/filepath" "strings" - "time" "sync" + "time" _ "github.com/mattn/go-sqlite3" // SQLite driver "heckel.io/ntfy/v2/log" @@ -283,8 +283,8 @@ var ( type messageCache struct { db *sql.DB queue *util.BatchingQueue[*message] - mu sync.Mutex nop bool + mu sync.Mutex } // newSqliteCache creates a SQLite file-backed cache diff --git a/server/message_cache_test.go b/server/message_cache_test.go index 778f28fe..f0a02b2e 100644 --- a/server/message_cache_test.go +++ b/server/message_cache_test.go @@ -3,8 +3,10 @@ package server import ( "database/sql" "fmt" + "github.com/stretchr/testify/assert" "net/netip" "path/filepath" + "sync" "testing" "time" @@ -90,6 +92,26 @@ func testCacheMessages(t *testing.T, c *messageCache) { require.Empty(t, messages) } +func TestSqliteCache_MessagesLock(t *testing.T) { + testCacheMessagesLock(t, newSqliteTestCache(t)) +} + +func TestMemCache_MessagesLock(t *testing.T) { + testCacheMessagesLock(t, newMemTestCache(t)) +} + +func testCacheMessagesLock(t *testing.T, c *messageCache) { + var wg sync.WaitGroup + for i := 0; i < 5000; i++ { + wg.Add(1) + go func() { + assert.Nil(t, c.AddMessage(newDefaultMessage("mytopic", "test message"))) + wg.Done() + }() + } + wg.Wait() +} + func TestSqliteCache_MessagesScheduled(t *testing.T) { testCacheMessagesScheduled(t, newSqliteTestCache(t)) } From 2d9e2356b1677d9db877ab86eb1bea462b030900 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 8 Aug 2025 16:13:39 -0400 Subject: [PATCH 4/5] Release notes --- docs/releases.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/releases.md b/docs/releases.md index 71097a78..20612a61 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1472,6 +1472,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release **Bug fixes + maintenance:** +* Add mutex around message cache writes to avoid `database locked` errors ([#1397](https://github.com/binwiederhier/ntfy/pull/1397), thanks to [@timofej673](https://github.com/timofej673)) * Add build tags `nopayments`, `nofirebase` and `nowebpush` to allow excluding external dependencies, useful for packaging in Debian ([#1420](https://github.com/binwiederhier/ntfy/pull/1420), discussion in [#1258](https://github.com/binwiederhier/ntfy/issues/1258), thanks to [@thekhalifa](https://github.com/thekhalifa) for packaging ntfy for Debian/Ubuntu) From ba86e08ffee7236584cb5c3232ac5f5f296dfe37 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 8 Aug 2025 16:19:02 -0400 Subject: [PATCH 5/5] Release notes --- docs/releases.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/releases.md b/docs/releases.md index 20612a61..facc9968 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1472,7 +1472,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release **Bug fixes + maintenance:** -* Add mutex around message cache writes to avoid `database locked` errors ([#1397](https://github.com/binwiederhier/ntfy/pull/1397), thanks to [@timofej673](https://github.com/timofej673)) +* Add mutex around message cache writes to avoid `database locked` errors ([#1397](https://github.com/binwiederhier/ntfy/pull/1397), [#1391](https://github.com/binwiederhier/ntfy/issues/1391), thanks to [@timofej673](https://github.com/timofej673)) * Add build tags `nopayments`, `nofirebase` and `nowebpush` to allow excluding external dependencies, useful for packaging in Debian ([#1420](https://github.com/binwiederhier/ntfy/pull/1420), discussion in [#1258](https://github.com/binwiederhier/ntfy/issues/1258), thanks to [@thekhalifa](https://github.com/thekhalifa) for packaging ntfy for Debian/Ubuntu)