Skip to content

Commit 9c53fc0

Browse files
committed
fix(thread-safety): eliminate races in log mutex, watcher, and index threads
- http_server: Replace lazy log mutex init (TOCTOU race between threads) with explicit cbm_ui_log_init() called from main before thread creation - watcher: Add cbm_mutex_t to protect projects hash table from concurrent access by main thread (watch/unwatch) and watcher thread (poll_once) - compat_thread: Add cbm_thread_detach() for POSIX and Windows - http_server: Detach index job threads to prevent handle leaks
1 parent 1d30971 commit 9c53fc0

File tree

6 files changed

+44
-5
lines changed

6 files changed

+44
-5
lines changed

src/foundation/compat_thread.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ int cbm_thread_join(cbm_thread_t *t) {
5959
return 0;
6060
}
6161

62+
int cbm_thread_detach(cbm_thread_t *t) {
63+
if (t->handle) {
64+
CloseHandle(t->handle);
65+
t->handle = NULL;
66+
}
67+
return 0;
68+
}
69+
6270
#else /* POSIX */
6371

6472
int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), void *arg) {
@@ -77,6 +85,10 @@ int cbm_thread_join(cbm_thread_t *t) {
7785
return pthread_join(t->handle, NULL);
7886
}
7987

88+
int cbm_thread_detach(cbm_thread_t *t) {
89+
return pthread_detach(t->handle);
90+
}
91+
8092
#endif
8193

8294
/* ── Mutex ────────────────────────────────────────────────────── */

src/foundation/compat_thread.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), v
3939
/* Wait for thread to finish. Returns 0 on success. */
4040
int cbm_thread_join(cbm_thread_t *t);
4141

42+
/* Detach thread so resources are freed on exit. Returns 0 on success. */
43+
int cbm_thread_detach(cbm_thread_t *t);
44+
4245
/* ── Mutex ────────────────────────────────────────────────────── */
4346

4447
#ifdef _WIN32

src/main.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@ int main(int argc, char **argv) {
307307
}
308308

309309
/* Create and start watcher in background thread */
310+
/* Initialize log mutex before any threads are created */
311+
cbm_ui_log_init();
312+
310313
cbm_store_t *watch_store = cbm_store_open_memory();
311314
g_watcher = cbm_watcher_new(watch_store, watcher_index_fn, NULL);
312315

src/ui/http_server.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,17 @@ static int g_log_count = 0;
142142
static cbm_mutex_t g_log_mutex;
143143
static atomic_int g_log_mutex_init = 0;
144144

145+
/* Must be called once before any threads are created. */
146+
void cbm_ui_log_init(void) {
147+
if (!atomic_exchange(&g_log_mutex_init, 1)) {
148+
cbm_mutex_init(&g_log_mutex);
149+
}
150+
}
151+
145152
/* Called from a log hook — appends a line to the ring buffer (thread-safe) */
146153
void cbm_ui_log_append(const char *line) {
147-
if (!line)
154+
if (!line || !atomic_load(&g_log_mutex_init))
148155
return;
149-
if (!atomic_load(&g_log_mutex_init)) {
150-
cbm_mutex_init(&g_log_mutex);
151-
atomic_store(&g_log_mutex_init, 1);
152-
}
153156
cbm_mutex_lock(&g_log_mutex);
154157
snprintf(g_log_ring[g_log_head], LOG_LINE_MAX, "%s", line);
155158
g_log_head = (g_log_head + 1) % LOG_RING_SIZE;
@@ -791,6 +794,7 @@ static void handle_index_start(struct mg_connection *c, struct mg_http_message *
791794
mg_http_reply(c, 500, g_cors_json, "{\"error\":\"thread creation failed\"}");
792795
return;
793796
}
797+
cbm_thread_detach(&tid); /* Don't leak thread handle */
794798

795799
mg_http_reply(c, 202, g_cors_json, "{\"status\":\"indexing\",\"slot\":%d,\"path\":\"%s\"}",
796800
slot, job->root_path);

src/ui/http_server.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ void cbm_http_server_run(cbm_http_server_t *srv);
3232
/* Check if the server started successfully (listener bound). */
3333
bool cbm_http_server_is_running(const cbm_http_server_t *srv);
3434

35+
/* Initialize the log ring buffer mutex. Must be called once before any threads. */
36+
void cbm_ui_log_init(void);
37+
3538
/* Append a log line to the UI ring buffer (called from log hook). */
3639
void cbm_ui_log_append(const char *line);
3740

src/watcher/watcher.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "foundation/log.h"
2121
#include "foundation/hash_table.h"
2222
#include "foundation/compat.h"
23+
#include "foundation/compat_thread.h"
2324
#include "foundation/compat_fs.h"
2425
#include "foundation/str_util.h"
2526

@@ -50,6 +51,7 @@ struct cbm_watcher {
5051
cbm_index_fn index_fn;
5152
void *user_data;
5253
CBMHashTable *projects; /* name → project_state_t* */
54+
cbm_mutex_t projects_lock;
5355
atomic_int stopped;
5456
};
5557

@@ -236,6 +238,7 @@ cbm_watcher_t *cbm_watcher_new(cbm_store_t *store, cbm_index_fn index_fn, void *
236238
w->index_fn = index_fn;
237239
w->user_data = user_data;
238240
w->projects = cbm_ht_create(CBM_SZ_32);
241+
cbm_mutex_init(&w->projects_lock);
239242
atomic_init(&w->stopped, 0);
240243
return w;
241244
}
@@ -244,8 +247,11 @@ void cbm_watcher_free(cbm_watcher_t *w) {
244247
if (!w) {
245248
return;
246249
}
250+
cbm_mutex_lock(&w->projects_lock);
247251
cbm_ht_foreach(w->projects, free_state_entry, NULL);
248252
cbm_ht_free(w->projects);
253+
cbm_mutex_unlock(&w->projects_lock);
254+
cbm_mutex_destroy(&w->projects_lock);
249255
free(w);
250256
}
251257

@@ -264,6 +270,7 @@ void cbm_watcher_watch(cbm_watcher_t *w, const char *project_name, const char *r
264270
}
265271

266272
/* Remove old entry first (key points to state's project_name) */
273+
cbm_mutex_lock(&w->projects_lock);
267274
project_state_t *old = cbm_ht_get(w->projects, project_name);
268275
if (old) {
269276
cbm_ht_delete(w->projects, project_name);
@@ -272,17 +279,22 @@ void cbm_watcher_watch(cbm_watcher_t *w, const char *project_name, const char *r
272279

273280
project_state_t *s = state_new(project_name, root_path);
274281
cbm_ht_set(w->projects, s->project_name, s);
282+
cbm_mutex_unlock(&w->projects_lock);
275283
cbm_log_info("watcher.watch", "project", project_name, "path", root_path);
276284
}
277285

278286
void cbm_watcher_unwatch(cbm_watcher_t *w, const char *project_name) {
279287
if (!w || !project_name) {
280288
return;
281289
}
290+
cbm_mutex_lock(&w->projects_lock);
282291
project_state_t *s = cbm_ht_get(w->projects, project_name);
283292
if (s) {
284293
cbm_ht_delete(w->projects, project_name);
285294
state_free(s);
295+
}
296+
cbm_mutex_unlock(&w->projects_lock);
297+
if (s) {
286298
cbm_log_info("watcher.unwatch", "project", project_name);
287299
}
288300
}
@@ -421,7 +433,9 @@ int cbm_watcher_poll_once(cbm_watcher_t *w) {
421433
.now = now_ns(),
422434
.reindexed = 0,
423435
};
436+
cbm_mutex_lock(&w->projects_lock);
424437
cbm_ht_foreach(w->projects, poll_project, &ctx);
438+
cbm_mutex_unlock(&w->projects_lock);
425439
return ctx.reindexed;
426440
}
427441

0 commit comments

Comments
 (0)