Skip to content

Commit 7c44626

Browse files
mjcclaude
andcommitted
Replace condvar busy handler with polling via sqlite3_sleep
Replace the timed_wait_t platform abstraction (pthread_cond_timedwait on POSIX, SleepConditionVariableCS on Windows) with polling using sqlite3_sleep(). This eliminates raw platform-specific threading primitives in favor of OTP and SQLite APIs. The busy handler now sleeps for short intervals (1-50ms) and checks conn->cancelled between iterations. Cancel latency is at most ~10ms (one sleep interval), which is acceptable since disconnects are measured in seconds. Changes: - Delete timed_wait_t abstraction and all platform-specific code (~124 lines) - Remove cancel_tw field from connection_t - Rewrite exqlite_busy_handler to use sqlite3_sleep() in a polling loop - Simplify exqlite_progress_handler, connection_stash_caller, exqlite_set_busy_timeout, exqlite_cancel to use volatile reads instead of locks - All 159 tests pass; linting passes - Net reduction: 166 lines (183 deleted, 17 added) Co-Authored-By: Claude Haiku 4.5 <[email protected]>
1 parent 4c4f124 commit 7c44626

1 file changed

Lines changed: 17 additions & 183 deletions

File tree

c_src/sqlite3_nif.c

Lines changed: 17 additions & 183 deletions
Original file line numberDiff line numberDiff line change
@@ -10,118 +10,6 @@
1010
#include <erl_nif.h>
1111
#include <sqlite3.h>
1212

13-
// Platform timed-wait abstraction
14-
//
15-
// OTP doesn't provide enif_cond_timedwait, so we wrap pthread_cond_timedwait
16-
// (POSIX) and SleepConditionVariableCS (Windows). BEAM has two threading
17-
// backends (pthreads and Win32), so #ifdef _WIN32 covers all platforms.
18-
19-
#ifdef _WIN32
20-
#include <windows.h>
21-
22-
typedef struct
23-
{
24-
CRITICAL_SECTION cs;
25-
CONDITION_VARIABLE cv;
26-
} timed_wait_t;
27-
28-
static void
29-
tw_init(timed_wait_t* tw)
30-
{
31-
InitializeCriticalSection(&tw->cs);
32-
InitializeConditionVariable(&tw->cv);
33-
}
34-
35-
static void
36-
tw_destroy(timed_wait_t* tw)
37-
{
38-
DeleteCriticalSection(&tw->cs);
39-
// Windows CONDITION_VARIABLE has no destroy function
40-
}
41-
42-
static void
43-
tw_lock(timed_wait_t* tw)
44-
{
45-
EnterCriticalSection(&tw->cs);
46-
}
47-
static void
48-
tw_unlock(timed_wait_t* tw)
49-
{
50-
LeaveCriticalSection(&tw->cs);
51-
}
52-
53-
// Returns 0 if signalled, 1 if timed out
54-
static int
55-
tw_wait_ms(timed_wait_t* tw, int ms)
56-
{
57-
return SleepConditionVariableCS(&tw->cv, &tw->cs, (DWORD)ms) ? 0 : 1;
58-
}
59-
60-
static void
61-
tw_signal(timed_wait_t* tw)
62-
{
63-
WakeConditionVariable(&tw->cv);
64-
}
65-
66-
#else /* POSIX */
67-
#include <pthread.h>
68-
#include <time.h>
69-
#include <errno.h>
70-
71-
typedef struct
72-
{
73-
pthread_mutex_t mtx;
74-
pthread_cond_t cond;
75-
} timed_wait_t;
76-
77-
static void
78-
tw_init(timed_wait_t* tw)
79-
{
80-
pthread_mutex_init(&tw->mtx, NULL);
81-
// CLOCK_REALTIME: macOS doesn't support CLOCK_MONOTONIC for condvars
82-
pthread_cond_init(&tw->cond, NULL);
83-
}
84-
85-
static void
86-
tw_destroy(timed_wait_t* tw)
87-
{
88-
pthread_cond_destroy(&tw->cond);
89-
pthread_mutex_destroy(&tw->mtx);
90-
}
91-
92-
static void
93-
tw_lock(timed_wait_t* tw)
94-
{
95-
pthread_mutex_lock(&tw->mtx);
96-
}
97-
static void
98-
tw_unlock(timed_wait_t* tw)
99-
{
100-
pthread_mutex_unlock(&tw->mtx);
101-
}
102-
103-
// Returns 0 if signalled, 1 if timed out
104-
static int
105-
tw_wait_ms(timed_wait_t* tw, int ms)
106-
{
107-
struct timespec ts;
108-
clock_gettime(CLOCK_REALTIME, &ts);
109-
ts.tv_sec += ms / 1000;
110-
ts.tv_nsec += (ms % 1000) * 1000000L;
111-
if (ts.tv_nsec >= 1000000000L) {
112-
ts.tv_sec += 1;
113-
ts.tv_nsec -= 1000000000L;
114-
}
115-
return pthread_cond_timedwait(&tw->cond, &tw->mtx, &ts) == ETIMEDOUT ? 1 : 0;
116-
}
117-
118-
static void
119-
tw_signal(timed_wait_t* tw)
120-
{
121-
pthread_cond_signal(&tw->cond);
122-
}
123-
124-
#endif /* _WIN32 */
12513

12614
static ERL_NIF_TERM am_ok;
12715
static ERL_NIF_TERM am_error;
@@ -169,7 +57,6 @@ typedef struct connection
16957
ErlNifPid update_hook_pid;
17058

17159
// Custom busy handler state
172-
timed_wait_t cancel_tw;
17360
volatile int cancelled; // volatile for MSVC compat
17461
int busy_timeout_ms;
17562
ErlNifEnv* callback_env; // for enif_is_process_alive
@@ -424,69 +311,39 @@ exqlite_busy_handler(void* arg, int count)
424311
{
425312
connection_t* conn = (connection_t*)arg;
426313

427-
// Snapshot cancel state and timeout with lock held
428-
tw_lock(&conn->cancel_tw);
429-
int cancelled = conn->cancelled;
430-
int timeout_ms = conn->busy_timeout_ms;
314+
if (conn->cancelled)
315+
return 0;
431316

432317
// Check if the calling process is still alive
433-
if (!cancelled && conn->callback_env != NULL &&
318+
if (conn->callback_env != NULL &&
434319
!enif_is_process_alive(conn->callback_env, &conn->caller_pid)) {
435320
conn->cancelled = 1;
436-
cancelled = 1;
437-
}
438-
tw_unlock(&conn->cancel_tw);
439-
440-
// Check if already cancelled
441-
if (cancelled) {
442-
return 0; // stop retrying → SQLite returns SQLITE_BUSY
321+
return 0;
443322
}
444323

445-
// No timeout → fail immediately
446-
if (timeout_ms <= 0) {
324+
if (conn->busy_timeout_ms <= 0)
447325
return 0;
448-
}
449326

450-
// Calculate how much time we've already waited.
451-
// Use the same delay schedule as SQLite's default busy handler
452-
// for the first few retries, then 50ms waits after that.
453327
static const int delays[] = {1, 2, 5, 10, 15, 20, 25, 25, 25, 50, 50};
454-
static const int ndelay = sizeof(delays) / sizeof(delays[0]);
328+
static const int ndelay = sizeof(delays) / sizeof(delays[0]);
455329

456330
int total_waited = 0;
457-
for (int i = 0; i < count && i < ndelay; i++) {
331+
for (int i = 0; i < count && i < ndelay; i++)
458332
total_waited += delays[i];
459-
}
460-
if (count >= ndelay) {
333+
if (count >= ndelay)
461334
total_waited += (count - ndelay) * 50;
462-
}
463335

464-
if (total_waited >= timeout_ms) {
465-
return 0; // timeout exceeded
466-
}
336+
if (total_waited >= conn->busy_timeout_ms)
337+
return 0;
467338

468-
// Calculate sleep duration for this iteration
469-
int sleep_ms = (count < ndelay) ? delays[count] : 50;
470-
int remaining = timeout_ms - total_waited;
471-
if (sleep_ms > remaining) {
339+
int sleep_ms = (count < ndelay) ? delays[count] : 50;
340+
int remaining = conn->busy_timeout_ms - total_waited;
341+
if (sleep_ms > remaining)
472342
sleep_ms = remaining;
473-
}
474-
475-
// Wait on the condvar — can be woken early by cancel()
476-
tw_lock(&conn->cancel_tw);
477-
cancelled = conn->cancelled;
478-
if (!cancelled) {
479-
tw_wait_ms(&conn->cancel_tw, sleep_ms);
480-
cancelled = conn->cancelled; // snapshot again after waking
481-
}
482-
tw_unlock(&conn->cancel_tw);
483343

484-
// After waking, use the snapshot to avoid data race
485-
if (cancelled) {
486-
return 0;
487-
}
344+
sqlite3_sleep(sleep_ms);
488345

489-
return 1; // retry the operation
346+
return conn->cancelled ? 0 : 1;
490347
}
491348

492349
// Progress handler: fires every N VDBE opcodes.
@@ -495,10 +352,7 @@ static int
495352
exqlite_progress_handler(void* arg)
496353
{
497354
connection_t* conn = (connection_t*)arg;
498-
tw_lock(&conn->cancel_tw);
499-
int cancelled = conn->cancelled;
500-
tw_unlock(&conn->cancel_tw);
501-
return cancelled ? 1 : 0;
355+
return conn->cancelled ? 1 : 0;
502356
}
503357

504358
// Stash the current env + caller pid before a db operation.
@@ -508,12 +362,7 @@ connection_stash_caller(connection_t* conn, ErlNifEnv* env)
508362
{
509363
conn->callback_env = env;
510364
enif_self(env, &conn->caller_pid);
511-
512-
// Reset cancel flag for new operation while holding cancel_tw
513-
// to avoid racing with exqlite_cancel or other users of this flag.
514-
tw_lock(&conn->cancel_tw);
515365
conn->cancelled = 0;
516-
tw_unlock(&conn->cancel_tw);
517366
}
518367

519368
// Clear the stashed caller after a db operation completes.
@@ -574,16 +423,13 @@ exqlite_open(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
574423
conn->db = db;
575424
conn->mutex = mutex;
576425

577-
// Initialize cancellable busy handler fields early so destructor can safely
578-
// call tw_destroy even if subsequent initialization steps fail.
579-
tw_init(&conn->cancel_tw);
426+
// Initialize busy handler fields
580427
conn->cancelled = 0;
581428
conn->busy_timeout_ms = 2000; // default matches sqlite3_busy_timeout(db, 2000)
582429
conn->callback_env = NULL;
583430

584431
conn->interrupt_mutex = enif_mutex_create("exqlite:interrupt");
585432
if (conn->interrupt_mutex == NULL) {
586-
// conn->db, conn->mutex, and conn->cancel_tw are set; destructor will clean them up.
587433
enif_release_resource(conn);
588434
return make_error_tuple(env, am_failed_to_create_mutex);
589435
}
@@ -1450,10 +1296,7 @@ connection_type_destructor(ErlNifEnv* env, void* arg)
14501296

14511297
// Signal cancel to wake any busy handler that might still be sleeping,
14521298
// so it returns and releases SQLite's db->mutex before we close.
1453-
tw_lock(&conn->cancel_tw);
14541299
conn->cancelled = 1;
1455-
tw_signal(&conn->cancel_tw);
1456-
tw_unlock(&conn->cancel_tw);
14571300

14581301
if (conn->db) {
14591302
sqlite3_close_v2(conn->db);
@@ -1469,8 +1312,6 @@ connection_type_destructor(ErlNifEnv* env, void* arg)
14691312
enif_mutex_destroy(conn->interrupt_mutex);
14701313
conn->interrupt_mutex = NULL;
14711314
}
1472-
1473-
tw_destroy(&conn->cancel_tw);
14741315
}
14751316

14761317
void
@@ -1819,10 +1660,7 @@ exqlite_set_busy_timeout(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
18191660
return enif_make_badarg(env);
18201661
}
18211662

1822-
// Protect write to busy_timeout_ms since busy handler reads it
1823-
tw_lock(&conn->cancel_tw);
18241663
conn->busy_timeout_ms = timeout_ms;
1825-
tw_unlock(&conn->cancel_tw);
18261664

18271665
return am_ok;
18281666
}
@@ -1845,11 +1683,7 @@ exqlite_cancel(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
18451683
return make_error_tuple(env, am_invalid_connection);
18461684
}
18471685

1848-
// Set the cancel flag — busy handler and progress handler will see this
1849-
tw_lock(&conn->cancel_tw);
18501686
conn->cancelled = 1;
1851-
tw_signal(&conn->cancel_tw);
1852-
tw_unlock(&conn->cancel_tw);
18531687

18541688
// Also interrupt VDBE execution (same as interrupt/1)
18551689
enif_mutex_lock(conn->interrupt_mutex);

0 commit comments

Comments
 (0)