Skip to content
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
77 changes: 39 additions & 38 deletions brain-bar/Sources/BrainBar/BrainBarApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,50 +73,51 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
createLegacyStatusItem()
}

DispatchQueue.global(qos: .userInitiated).async { [weak self] in
guard let self else { return }

let dbPath = BrainBarServer.defaultDBPath()
NSLog("[BrainBar] Opening database at %@", dbPath)
let sharedDatabase = BrainDatabase(path: dbPath)

let server = BrainBarServer(database: sharedDatabase)
server.onDatabaseReady = { [weak self] database in
Task { @MainActor in
self?.configureQuickCapture(database: database)
}
let dbPath = BrainBarServer.defaultDBPath()
NSLog("[BrainBar] Starting server before database readiness at %@", dbPath)
let server = BrainBarServer(dbPath: dbPath)
server.onStartRejected = { reason in
NSLog("[BrainBar] Startup rejected: %@", reason)
Task { @MainActor in
NSApp.terminate(nil)
}

DispatchQueue.main.async { [weak self] in
}
server.onDatabaseReady = { [weak self] database in
Task { @MainActor in
guard let self else { return }
let collector = StatsCollector(
dbPath: dbPath,
daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier)
)
let injectionStore = try? InjectionStore(databasePath: dbPath)

self.sharedDatabase = sharedDatabase
self.server = server
self.collector = collector
self.injectionStore = injectionStore

server.start()
runtime.install(
collector: collector,
injectionStore: injectionStore,
database: sharedDatabase
self.sharedDatabase = database
self.configureQuickCapture(database: database)
self.runtime.install(
collector: self.collector ?? StatsCollector(
dbPath: dbPath,
daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier)
),
injectionStore: self.injectionStore,
database: database
)
flushPendingBrainBarURLs()
self.flushPendingBrainBarURLs()
}
}

if launchMode == .legacyStatusItem {
installLegacyMenuBarSurface(with: collector)
}
let collector = StatsCollector(
dbPath: dbPath,
daemonMonitor: DaemonHealthMonitor(targetPID: ProcessInfo.processInfo.processIdentifier)
)
let injectionStore = try? InjectionStore(databasePath: dbPath)

collector.start()
configureQuickCaptureHotkey()
NSLog("[BrainBar] Backend ready — launchMode=%@", String(describing: launchMode))
}
self.server = server
self.collector = collector
self.injectionStore = injectionStore

server.start()

if launchMode == .legacyStatusItem {
installLegacyMenuBarSurface(with: collector)
}

collector.start()
configureQuickCaptureHotkey()
NSLog("[BrainBar] Socket ready; database will self-heal — launchMode=%@", String(describing: launchMode))
Comment on lines +112 to +120
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Don't start app side effects before startup acceptance is known.

server.start() returns before the lock check runs on the server queue, so a losing duplicate instance still installs the legacy surface, starts the collector, and registers the hotkey before onStartRejected gets back onto the main actor. That briefly recreates the duplicate side effects this PR is trying to eliminate. Gate this work behind an explicit successful-start callback, or make startup acceptance synchronous.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Sources/BrainBar/BrainBarApp.swift` around lines 112 - 120,
server.start() returns before the server's lock check completes causing
installLegacyMenuBarSurface(with:), collector.start(), and
configureQuickCaptureHotkey() to run even for rejected duplicates; change the
flow to only perform those side effects after an explicit success callback from
the server (e.g. a completion handler on server.start or observing an
onStartAccepted/onStartRejected event) — move calls to
installLegacyMenuBarSurface(with: collector), collector.start(),
configureQuickCaptureHotkey(), and the NSLog trailing message into the
success/accepted path and ensure the rejected path (onStartRejected) does not
run them.

}

func applicationWillTerminate(_ notification: Notification) {
Expand Down
66 changes: 66 additions & 0 deletions brain-bar/Sources/BrainBar/BrainBarInstanceLock.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import Darwin
import Foundation
import os

final class BrainBarInstanceLock: @unchecked Sendable {
enum AcquireError: Error, Equatable {
case alreadyRunning
case openFailed(String)
case lockFailed(String)
}

private let fd: Int32
private let lockPath: String
private let releaseLock = OSAllocatedUnfairLock(initialState: false)

private init(fd: Int32, lockPath: String) {
self.fd = fd
self.lockPath = lockPath
}

deinit {
release()
}

static func acquire(lockPath: String) throws -> BrainBarInstanceLock {
let directory = URL(fileURLWithPath: lockPath).deletingLastPathComponent()
try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)

let fd = open(lockPath, O_RDWR | O_CREAT, 0o600)
guard fd >= 0 else {
throw AcquireError.openFailed(String(cString: strerror(errno)))
}

guard flock(fd, LOCK_EX | LOCK_NB) == 0 else {
let lockErrno = errno
let message = String(cString: strerror(lockErrno))
close(fd)
if lockErrno == EWOULDBLOCK {
throw AcquireError.alreadyRunning
}
throw AcquireError.lockFailed(message)
}

var pidLine = "\(getpid())\n".data(using: .utf8) ?? Data()
ftruncate(fd, 0)
lseek(fd, 0, SEEK_SET)
_ = pidLine.withUnsafeMutableBytes { ptr in
write(fd, ptr.baseAddress, ptr.count)
}
fsync(fd)

return BrainBarInstanceLock(fd: fd, lockPath: lockPath)
}

func release() {
let shouldRelease = releaseLock.withLock { released -> Bool in
guard !released else { return false }
released = true
return true
}
guard shouldRelease else { return }
flock(fd, LOCK_UN)
close(fd)
try? FileManager.default.removeItem(atPath: lockPath)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep lockfile inode stable while another instance may acquire

release() unlinks lockPath after unlocking, which breaks flock-based singleton guarantees during handoff: if a second process acquires the lock between LOCK_UN and removeItem, this code can delete the inode that process is locking, allowing a third process to recreate the path and acquire a separate lock concurrently. For flock locks, the lock file should remain in place (or at least not be removed in this sequence) to avoid split-brain ownership.

Useful? React with 👍 / 👎.

Comment on lines +55 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the BrainBarInstanceLock.swift file
find . -name "BrainBarInstanceLock.swift" -type f

Repository: EtanHey/brainlayer

Length of output: 118


🏁 Script executed:

# Search for the BrainBarInstanceLock file more broadly
fd -e swift -i brainbarinstancelock

Repository: EtanHey/brainlayer

Length of output: 116


🏁 Script executed:

# List the brain-bar sources directory structure
fd . brain-bar/Sources/ -type f -name "*.swift" | head -20

Repository: EtanHey/brainlayer

Length of output: 233


🏁 Script executed:

# Read the BrainBarInstanceLock.swift file
cat -n ./brain-bar/Sources/BrainBar/BrainBarInstanceLock.swift

Repository: EtanHey/brainlayer

Length of output: 2504


🏁 Script executed:

# Check for any related tests or usage patterns
rg "BrainBarInstanceLock" --type swift -A 3 -B 3

Repository: EtanHey/brainlayer

Length of output: 4530


🏁 Script executed:

# Search for any initialization or creation logic that shows the lock design intent
rg "flock\|lockPath\|removeItem" brain-bar/Sources/BrainBar/BrainBarInstanceLock.swift -A 2 -B 2

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check if there are any issues, PRs, or comments about this race condition
rg "BrainBarInstanceLock|instance.*lock|race" --type swift brain-bar/ -i | head -30

Repository: EtanHey/brainlayer

Length of output: 2762


🏁 Script executed:

# Look at the test more closely to see if it covers the race condition scenario
cat -n brain-bar/Tests/BrainBarTests/BrainBarReliabilityTests.swift | grep -A 20 "BrainBarInstanceLock"

Repository: EtanHey/brainlayer

Length of output: 1308


🏁 Script executed:

# Search for any documentation or comments about why removeItem is there
rg "removeItem|cleanup|delete.*lock" brain-bar/Sources/BrainBar/ -B 3 -A 3

Repository: EtanHey/brainlayer

Length of output: 4156


Remove the lock file deletion to prevent a race condition that breaks single-instance guarantees.

The flock syscall attaches locks to inodes, not pathnames. Between close(fd) and removeItem(lockPath), another process can open the same path and acquire the lock on the same inode. A third process can then create a new file at that path before the first process completes deletion, gaining a lock on a different inode. This allows two processes to hold locks simultaneously on different inodes, violating the single-instance invariant.

🔧 Suggested fix
     func release() {
         let shouldRelease = releaseLock.withLock { released -> Bool in
             guard !released else { return false }
             released = true
             return true
         }
         guard shouldRelease else { return }
         flock(fd, LOCK_UN)
         close(fd)
-        try? FileManager.default.removeItem(atPath: lockPath)
     }

The lock file can safely remain on disk; it acts as a marker and causes no harm on subsequent acquisitions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Sources/BrainBar/BrainBarInstanceLock.swift` around lines 55 - 64,
The release() method currently unlinks the lock file after releasing the flock,
which can cause a race allowing multiple processes to hold locks on different
inodes; remove the FileManager.default.removeItem(atPath: lockPath) call and
keep only the unlock (flock(fd, LOCK_UN)) and close(fd) steps (preserving the
releaseLock guard logic) so the lock file remains on disk as a marker and
prevents the race.

}
}
61 changes: 55 additions & 6 deletions brain-bar/Sources/BrainBar/BrainBarServer.swift
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low

private func handleMessage(fd: Int32, request: [String: Any]) -> [String: Any] {

handleMessage intercepts brain_subscribe, brain_unsubscribe, and brain_ack before they reach MCPRouter.handleToolsCall, skipping the validate(arguments:for:) check. This lets clients send oversized agent_id strings or unbounded tags arrays that bypass the maxLength/maxItems limits defined in limitedInputSchema, risking database storage failures or memory pressure.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/BrainBarServer.swift around line 417:

`handleMessage` intercepts `brain_subscribe`, `brain_unsubscribe`, and `brain_ack` before they reach `MCPRouter.handleToolsCall`, skipping the `validate(arguments:for:)` check. This lets clients send oversized `agent_id` strings or unbounded `tags` arrays that bypass the `maxLength`/`maxItems` limits defined in `limitedInputSchema`, risking database storage failures or memory pressure.

Evidence trail:
BrainBarServer.swift lines 417-425: handleMessage intercepts brain_subscribe/brain_unsubscribe/brain_ack before router.handle(). MCPRouter.swift line 148: validate(arguments:for:) is called in the router path. MCPRouter.swift lines 703-762: validate() enforces maxLength/maxItems from schema. MCPRouter.swift lines 659-700: limitedInputSchema adds default maxLength/maxItems. MCPRouter.swift lines 988-1030: brain_subscribe/brain_unsubscribe/brain_ack schemas use limitedInputSchema. BrainBarServer.swift lines 551-640: handleSubscribeTool/handleUnsubscribeTool/handleAckTool have no validation of argument sizes.

Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ final class BrainBarServer: @unchecked Sendable {
private let dbPath: String
private let providedDatabase: BrainDatabase?
private let databaseRecoveryPolicy: DatabaseRecoveryPolicy
private let instanceLockPath: String
private let queue = DispatchQueue(label: "com.brainlayer.brainbar.server", qos: .userInitiated)
private var listenFD: Int32 = -1
private var listenSource: DispatchSourceRead?
Expand All @@ -99,7 +100,10 @@ final class BrainBarServer: @unchecked Sendable {
private var database: BrainDatabase!
private var databaseRetryWorkItem: DispatchWorkItem?
private var lastDatabaseRetryDelayMillis: UInt64?
private var databaseOpenInProgress = false
private var instanceLock: BrainBarInstanceLock?
var onDatabaseReady: (@Sendable (BrainDatabase) -> Void)?
var onStartRejected: (@Sendable (String) -> Void)?
/// Maximum EAGAIN retries before disconnecting a stalled client.
/// Each retry sleeps 1ms, so 10 retries = 10ms max blocking the serial queue.
static let maxWriteRetries = 10
Expand Down Expand Up @@ -137,12 +141,14 @@ final class BrainBarServer: @unchecked Sendable {
socketPath: String? = nil,
dbPath: String? = nil,
database: BrainDatabase? = nil,
databaseRecoveryPolicy: DatabaseRecoveryPolicy = DatabaseRecoveryPolicy()
databaseRecoveryPolicy: DatabaseRecoveryPolicy = DatabaseRecoveryPolicy(),
instanceLockPath: String? = nil
) {
self.socketPath = socketPath ?? Self.defaultSocketPath()
self.dbPath = dbPath ?? Self.defaultDBPath()
providedDatabase = database
self.databaseRecoveryPolicy = databaseRecoveryPolicy
self.instanceLockPath = instanceLockPath ?? Self.defaultInstanceLockPath(socketPath: self.socketPath)
}

static func defaultSocketPath() -> String {
Expand All @@ -162,6 +168,10 @@ final class BrainBarServer: @unchecked Sendable {
return "\(home)/.local/share/brainlayer/brainlayer.db"
}

static func defaultInstanceLockPath(socketPath: String = defaultSocketPath()) -> String {
"\(socketPath).lock"
}

func start() {
queue.async { [weak self] in
self?.startOnQueue()
Expand All @@ -175,6 +185,18 @@ final class BrainBarServer: @unchecked Sendable {
}

private func startOnQueue() {
do {
instanceLock = try BrainBarInstanceLock.acquire(lockPath: instanceLockPath)
} catch BrainBarInstanceLock.AcquireError.alreadyRunning {
NSLog("[BrainBar] Another BrainBar instance owns %@. Exiting this server.", instanceLockPath)
onStartRejected?("another BrainBar instance owns \(instanceLockPath)")
return
} catch {
NSLog("[BrainBar] Failed to acquire single-instance lock %@: %@", instanceLockPath, String(describing: error))
onStartRejected?("failed to acquire single-instance lock \(instanceLockPath): \(error)")
return
}
Comment on lines +188 to +198
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Release the singleton lock on every startup failure after this point.

Once instanceLock is acquired here, later startup failures still just return. That leaves this process holding the lock and never invokes onStartRejected, so the app can stay alive with no listener while blocking all future launches until exit. Add a shared failure path that releases the lock and reports rejection on every post-acquire early return.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Sources/BrainBar/BrainBarServer.swift` around lines 188 - 198,
After successfully acquiring instanceLock via BrainBarInstanceLock.acquire,
ensure the lock is always released and onStartRejected is invoked for any
subsequent early-return failure paths: introduce a deferred cleanup (e.g., a
defer block or shared failure handler) immediately after assignment of
instanceLock that checks a startupSucceeded flag (set true only on full
successful startup) and if false calls instanceLock.release (or the appropriate
BrainBarInstanceLock release method) and invokes onStartRejected with the
existing error message; update all post-acquire early returns to rely on this
shared cleanup so the lock is not left held when startup fails.


// 1. Create router FIRST (no DB dependency).
// initialize + tools/list work without a database.
router = MCPRouter()
Expand Down Expand Up @@ -253,12 +275,36 @@ final class BrainBarServer: @unchecked Sendable {
}

private func attemptDatabaseOpen() {
guard database == nil else { return }
guard database == nil, !databaseOpenInProgress else { return }

if let providedDatabase {
finishDatabaseOpen(providedDatabase)
return
}

databaseOpenInProgress = true
let dbPath = self.dbPath
let busyTimeoutMillis = databaseRecoveryPolicy.busyTimeoutMillis
DispatchQueue.global(qos: .userInitiated).async { [weak self] in
let db = BrainDatabase(
path: dbPath,
openConfiguration: .init(busyTimeoutMillis: busyTimeoutMillis)
)
self?.queue.async { [weak self] in
self?.databaseOpenInProgress = false
self?.finishDatabaseOpen(db)
}
}
}

private func finishDatabaseOpen(_ db: BrainDatabase) {
guard listenSource != nil else {
if providedDatabase == nil {
db.close()
}
return
}

let db = providedDatabase ?? BrainDatabase(
path: dbPath,
openConfiguration: .init(busyTimeoutMillis: databaseRecoveryPolicy.busyTimeoutMillis)
)
if db.isOpen {
databaseRetryWorkItem?.cancel()
databaseRetryWorkItem = nil
Expand Down Expand Up @@ -496,6 +542,9 @@ final class BrainBarServer: @unchecked Sendable {
database?.close()
}
database = nil
databaseOpenInProgress = false
instanceLock?.release()
instanceLock = nil
NSLog("[BrainBar] Server stopped")
}

Expand Down
48 changes: 46 additions & 2 deletions brain-bar/Sources/BrainBar/BrainDatabase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ final class BrainDatabase: @unchecked Sendable {
let importance: Int
}

enum StoreWriteOutcome: Sendable {
case stored(StoredChunk)
case queued
}

struct PendingStoreItem: Codable, Sendable {
let content: String
let tags: [String]
Expand Down Expand Up @@ -702,7 +707,9 @@ final class BrainDatabase: @unchecked Sendable {
importance: Int,
source: String,
queueID: String? = nil,
refreshStatistics: Bool = true
refreshStatistics: Bool = true,
retries: Int = 3,
busyTimeoutMillis: Int32? = nil
) throws -> StoredChunk {
guard let db else { throw DBError.notOpen }
let chunkID = "brainbar-\(UUID().uuidString.lowercased().prefix(12))"
Expand All @@ -712,7 +719,17 @@ final class BrainDatabase: @unchecked Sendable {
INSERT INTO chunks (id, content, metadata, source_file, tags, importance, source, content_type, char_count, preview_text)
VALUES (?, ?, ?, 'brainbar-store', ?, ?, ?, 'user_message', ?, ?)
"""
try runWriteStatement(on: db, sql: sql, retries: 3) { stmt in
let previousBusyTimeout = busyTimeoutMillis.flatMap { _ in queryPragma(db, name: "busy_timeout") }
if let busyTimeoutMillis {
try executeOnHandle(db, sql: "PRAGMA busy_timeout = \(max(1, busyTimeoutMillis))")
}
defer {
if let previousBusyTimeout {
try? executeOnHandle(db, sql: "PRAGMA busy_timeout = \(previousBusyTimeout)")
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Busy timeout permanently lowered if pragma query fails

Medium Severity

The store() method's defer block only restores the previous busy_timeout when previousBusyTimeout is non-nil. Since previousBusyTimeout is computed via busyTimeoutMillis.flatMap { _ in queryPragma(…) }, it becomes nil if queryPragma fails — yet the PRAGMA is still changed to the low budget value (e.g., 50ms). In that case, the defer skips restoration and the shared database connection keeps the 50ms timeout permanently. All subsequent write operations under any contention would then fail far too quickly instead of using the configured 30-second timeout.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d059427. Configure here.

Comment on lines +722 to +730
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the BrainDatabase.swift file around the flagged lines
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '710,750p'

Repository: EtanHey/brainlayer

Length of output: 2224


🏁 Script executed:

# Check the broader context of the store method and how db is used
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '700,780p'

Repository: EtanHey/brainlayer

Length of output: 3771


🏁 Script executed:

# Look for the BrainDatabase class definition and how the db handle is initialized
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | head -100

Repository: EtanHey/brainlayer

Length of output: 4353


🏁 Script executed:

# Check if there are any synchronization primitives (locks, queues, etc.) around the db handle
rg -n "nonisolated\|DispatchQueue\|Lock\|Mutex\|serial" brain-bar/Sources/BrainBar/BrainDatabase.swift | head -30

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Search for other callers of the store method to understand concurrency context
rg -n "\.store\(" brain-bar/Sources/BrainBar/ -A 2 | head -40

Repository: EtanHey/brainlayer

Length of output: 1921


🏁 Script executed:

# Check if there's any synchronization around db handle access or SQLite connection management
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '150,250p'

Repository: EtanHey/brainlayer

Length of output: 3860


🏁 Script executed:

# Look for the db property definition
rg -n "var db\|let db\|private.*db" brain-bar/Sources/BrainBar/BrainDatabase.swift | head -20

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check for any dispatch queue or serial access patterns in the class
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '1,150p' | tail -50

Repository: EtanHey/brainlayer

Length of output: 2375


🏁 Script executed:

# Look at how storeAsync is implemented to understand threading model
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '778,800p'

Repository: EtanHey/brainlayer

Length of output: 1297


🏁 Script executed:

# Check the open() method to see how db connection is established and managed
rg -n "func open\|private func open" brain-bar/Sources/BrainBar/BrainDatabase.swift -A 15 | head -40

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check for any actor or main thread enforcement
rg -n "@MainActor\|@globalActor\|nonisolated" brain-bar/Sources/BrainBar/BrainDatabase.swift

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check if there's a serial queue or other synchronization I missed
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '239,300p'

Repository: EtanHey/brainlayer

Length of output: 3080


🏁 Script executed:

# Look for any queue or lock protecting db access
rg -n "serialQueue\|dbQueue\|dbLock\|synchronized" brain-bar/Sources/BrainBar/BrainDatabase.swift

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check the runWriteStatement implementation to see if it has synchronization
rg -n "func runWriteStatement\|private func runWriteStatement" brain-bar/Sources/BrainBar/BrainDatabase.swift -A 10 | head -30

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Verify SQLite PRAGMA busy_timeout behavior
web_search
SQLite PRAGMA busy_timeout connection state per connection or global

Repository: EtanHey/brainlayer

Length of output: 158


🏁 Script executed:

# Check if there are any other store() calls that happen concurrently
rg -n "\.store\(|storeAsync|storeOrQueueWithinBudget" brain-bar/Sources/BrainBar/ --type swift | wc -l

Repository: EtanHey/brainlayer

Length of output: 65


🏁 Script executed:

# Check the full threading model - is db protected by any queue?
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '239,400p'

Repository: EtanHey/brainlayer

Length of output: 7265


🏁 Script executed:

# Check if runWriteStatement or similar has any protection
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '850,950p'

Repository: EtanHey/brainlayer

Length of output: 4988


🏁 Script executed:

# Look for queryPragma and executeOnHandle to understand what they do
rg -n "func queryPragma|func executeOnHandle" brain-bar/Sources/BrainBar/BrainDatabase.swift -A 5

Repository: EtanHey/brainlayer

Length of output: 757


🏁 Script executed:

# Verify that there's no synchronized access pattern I'm missing
rg -n "DispatchQueue\|serial\|queue\|Lock\|Mutex" brain-bar/Sources/BrainBar/BrainDatabase.swift | head -10

Repository: EtanHey/brainlayer

Length of output: 44


🌐 Web query:

SQLite PRAGMA busy_timeout connection-specific thread-safe

💡 Result:

Yes, the SQLite PRAGMA busy_timeout is connection-specific (per-database connection) and operates safely in thread-safe builds of SQLite. Key points: - It sets the busy handler for that specific database connection only. Each connection has its own independent busy handler/timeout setting. Setting it on one connection does not affect others. - Default is 0 (no timeout, immediate SQLITE_BUSY return). - Use: PRAGMA busy_timeout = milliseconds; (e.g., PRAGMA busy_timeout = 5000; for 5 seconds) - Equivalent C API: sqlite3_busy_timeout(sqlite3 *db, int ms); explicitly per connection. - Thread safety: Works in multi-threaded (SQLITE_THREADSAFE=2) and serialized (default, SQLITE_THREADSAFE=1) modes. In serialized mode, full mutex protection serializes access to the connection. In multi-thread, connections must not be shared across threads without external synchronization, but the timeout itself is a simple per-connection value. Compile with thread-safe options for multi-threaded use; single-thread mode (SQLITE_THREADSAFE=0) omits mutexes but still supports per-connection timeouts if not multi-threaded. - Must be set after opening each connection, as PRAGMAs are session/connection-specific and do not persist. - In WAL mode (recommended for concurrency), it helps handle lock contention between the single writer and multiple readers. Note: Timeouts may not apply in deadlocks (immediate SQLITE_BUSY) or certain recovery scenarios.

Citations:


🏁 Script executed:

# Check how storeAsync and other concurrent paths use the db
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '779,790p'

Repository: EtanHey/brainlayer

Length of output: 731


🏁 Script executed:

# Verify there's truly no serial queue or synchronization by looking at the full class structure
rg -n "private.*queue\|private.*lock\|private.*serial" brain-bar/Sources/BrainBar/BrainDatabase.swift

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check if any of the store callers use main thread enforcement
rg -n "db.store\|db.storeAsync\|db.storeOrQueue" brain-bar/Sources/BrainBar/QuickCaptureController.swift -B 3 -A 3 | head -20

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Check configureConnection to see if busy_timeout is set during init
rg -n "func configureConnection|func openConnection" brain-bar/Sources/BrainBar/BrainDatabase.swift -A 15 | head -40

Repository: EtanHey/brainlayer

Length of output: 1836


🏁 Script executed:

# Check if the initial OpenConfiguration.busyTimeoutMillis is applied at all
rg -n "openConfiguration\|busyTimeoutMillis" brain-bar/Sources/BrainBar/BrainDatabase.swift | head -20

Repository: EtanHey/brainlayer

Length of output: 44


🏁 Script executed:

# Look at executeOnHandle more completely to confirm it's not synchronized
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '1642,1660p'

Repository: EtanHey/brainlayer

Length of output: 985


🏁 Script executed:

# Let me check if there are any other clues about thread safety in the broader context
cat -n brain-bar/Sources/BrainBar/BrainDatabase.swift | sed -n '1,50p'

Repository: EtanHey/brainlayer

Length of output: 2346


🏁 Script executed:

# Verify the SQLITE_OPEN_FULLMUTEX flag means SQLite serializes access internally
# But that doesn't prevent the caller from having race conditions in PRAGMA mutation
rg -n "FULLMUTEX\|Sendable" brain-bar/Sources/BrainBar/BrainDatabase.swift | head -10

Repository: EtanHey/brainlayer

Length of output: 44


Concurrent PRAGMA mutations on shared handle can cause operations to inherit wrong timeouts.

The code mutates busy_timeout on the shared SQLite connection and restores it via defer, but since storeAsync dispatches to a concurrent global queue and there is no external synchronization on the db handle, concurrent store() calls can interleave. One thread may set the timeout to 50 ms while another thread reads that value before restoration, causing unrelated operations to use incorrect timeout values or fail spuriously. Use a dedicated short-timeout connection for budgeted writes instead of mutating the shared handle.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@brain-bar/Sources/BrainBar/BrainDatabase.swift` around lines 722 - 730, The
current code mutates the shared SQLite handle's PRAGMA busy_timeout via
busyTimeoutMillis/queryPragma/executeOnHandle which races when
storeAsync/store() run concurrently; instead, when a custom busyTimeoutMillis is
requested create and use a dedicated short-lived DB connection/handle for that
budgeted write (set its PRAGMA busy_timeout once on the new handle, run the
write via executeOnHandle or equivalent, then close the handle) rather than
changing and restoring the shared connection; update the logic around
busyTimeoutMillis, previousBusyTimeout, queryPragma, and executeOnHandle to
open/close a temporary connection for timed writes to eliminate interleaving on
the shared handle.


try runWriteStatement(on: db, sql: sql, retries: retries) { stmt in
bindText(chunkID, to: stmt, index: 1)
bindText(content, to: stmt, index: 2)
bindText(metadataJSON, to: stmt, index: 3)
Expand All @@ -731,6 +748,33 @@ final class BrainDatabase: @unchecked Sendable {
return StoredChunk(chunkID: chunkID, rowID: rowID)
}

func storeOrQueueWithinBudget(
content: String,
tags: [String],
importance: Int,
source: String,
busyTimeoutMillis: Int32 = 50
) throws -> StoreWriteOutcome {
do {
let stored = try store(
content: content,
tags: tags,
importance: importance,
source: source,
refreshStatistics: true,
retries: 0,
busyTimeoutMillis: busyTimeoutMillis
)
return .stored(stored)
} catch {
guard shouldQueueStoreError(error) else {
throw error
}
try queuePendingStore(content: content, tags: tags, importance: importance, source: source)
return .queued
}
}

/// Async wrapper for store() — runs DB write off the main thread.
func storeAsync(content: String, tags: [String], importance: Int, source: String) async throws -> StoredChunk {
try await withCheckedThrowingContinuation { continuation in
Expand Down
10 changes: 3 additions & 7 deletions brain-bar/Sources/BrainBar/MCPRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ final class MCPRouter: @unchecked Sendable {
guard let db = database else {
throw ToolError.noDatabase
}
do {
let stored = try db.store(content: content, tags: tags, importance: importance, source: "mcp")
switch try db.storeOrQueueWithinBudget(content: content, tags: tags, importance: importance, source: "mcp") {
case .stored(let stored):
let flushedStores = db.flushPendingStores()
return ToolOutput(
text: Formatters.formatStoreResult(chunkId: stored.chunkID),
Expand All @@ -308,11 +308,7 @@ final class MCPRouter: @unchecked Sendable {
}
]
)
} catch {
guard db.shouldQueueStoreError(error) else {
throw error
}
try db.queuePendingStore(content: content, tags: tags, importance: importance, source: "mcp")
case .queued:
return ToolOutput(
text: Formatters.formatStoreResult(chunkId: "", queued: true),
metadata: ["queued": true]
Expand Down
Loading
Loading