Skip to content
Open
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
23 changes: 22 additions & 1 deletion src/foundation/compat_thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ int cbm_thread_join(cbm_thread_t *t) {
return CBM_NOT_FOUND;
}
CloseHandle(t->handle);
t->handle = NULL;
return 0;
}

int cbm_thread_detach(cbm_thread_t *t) {
if (t->handle) {
CloseHandle(t->handle);
t->handle = NULL;
}
return 0;
}
Comment on lines +63 to +69
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The POSIX implementation doesn’t clear/poison the stored pthread_t after a successful detach, which makes accidental later join()/double-detach harder to detect and can lead to inconsistent lifecycle handling across platforms (Windows clears the handle). Consider, on success, updating the thread object to a known 'no handle' state (whatever is idiomatic for cbm_thread_t on POSIX), and/or documenting that cbm_thread_join() must not be called after detach.

Copilot uses AI. Check for mistakes.

Expand All @@ -74,7 +83,19 @@ int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), v
}

int cbm_thread_join(cbm_thread_t *t) {
return pthread_join(t->handle, NULL);
int rc = pthread_join(t->handle, NULL);
if (rc == 0) {
memset(&t->handle, 0, sizeof(t->handle));
}
return rc;
}

int cbm_thread_detach(cbm_thread_t *t) {
int rc = pthread_detach(t->handle);
if (rc == 0) {
memset(&t->handle, 0, sizeof(t->handle));
}
return rc;
}

#endif
Expand Down
3 changes: 3 additions & 0 deletions src/foundation/compat_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), v
/* Wait for thread to finish. Returns 0 on success. */
int cbm_thread_join(cbm_thread_t *t);

/* Detach thread so resources are freed on exit. Returns 0 on success. */
int cbm_thread_detach(cbm_thread_t *t);

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

#ifdef _WIN32
Expand Down
3 changes: 3 additions & 0 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ int main(int argc, char **argv) {
}

/* Create and start watcher in background thread */
/* Initialize log mutex before any threads are created */
cbm_ui_log_init();

cbm_store_t *watch_store = cbm_store_open_memory();
g_watcher = cbm_watcher_new(watch_store, watcher_index_fn, NULL);

Expand Down
36 changes: 31 additions & 5 deletions src/ui/http_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,41 @@ static char g_log_ring[LOG_RING_SIZE][LOG_LINE_MAX];
static int g_log_head = 0;
static int g_log_count = 0;
static cbm_mutex_t g_log_mutex;
static atomic_int g_log_mutex_init = 0;

enum {
CBM_LOG_MUTEX_UNINIT = 0,
CBM_LOG_MUTEX_INITING = 1,
CBM_LOG_MUTEX_INITED = 2
};
static atomic_int g_log_mutex_init = CBM_LOG_MUTEX_UNINIT;

/* Safe for concurrent callers: only publishes INITED after cbm_mutex_init()
* has completed. Callers that lose the CAS race spin until init finishes. */
void cbm_ui_log_init(void) {
int state = atomic_load(&g_log_mutex_init);
if (state == CBM_LOG_MUTEX_INITED)
return;

state = CBM_LOG_MUTEX_UNINIT;
if (atomic_compare_exchange_strong(&g_log_mutex_init, &state, CBM_LOG_MUTEX_INITING)) {
cbm_mutex_init(&g_log_mutex);
atomic_store(&g_log_mutex_init, CBM_LOG_MUTEX_INITED);
return;
}

/* Another thread is initializing — spin until done */
while (atomic_load(&g_log_mutex_init) != CBM_LOG_MUTEX_INITED) {
cbm_usleep(1000); /* 1ms */
}
}

/* Called from a log hook — appends a line to the ring buffer (thread-safe) */
void cbm_ui_log_append(const char *line) {
if (!line)
return;
if (!atomic_load(&g_log_mutex_init)) {
cbm_mutex_init(&g_log_mutex);
atomic_store(&g_log_mutex_init, 1);
}
/* Ensure mutex is initialized (safe for early single-threaded logging
* and concurrent calls via atomic_exchange once-init pattern). */
cbm_ui_log_init();
cbm_mutex_lock(&g_log_mutex);
snprintf(g_log_ring[g_log_head], LOG_LINE_MAX, "%s", line);
g_log_head = (g_log_head + 1) % LOG_RING_SIZE;
Expand Down Expand Up @@ -791,6 +816,7 @@ static void handle_index_start(struct mg_connection *c, struct mg_http_message *
mg_http_reply(c, 500, g_cors_json, "{\"error\":\"thread creation failed\"}");
return;
}
cbm_thread_detach(&tid); /* Don't leak thread handle */

mg_http_reply(c, 202, g_cors_json, "{\"status\":\"indexing\",\"slot\":%d,\"path\":\"%s\"}",
slot, job->root_path);
Expand Down
3 changes: 3 additions & 0 deletions src/ui/http_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ void cbm_http_server_run(cbm_http_server_t *srv);
/* Check if the server started successfully (listener bound). */
bool cbm_http_server_is_running(const cbm_http_server_t *srv);

/* Initialize the log ring buffer mutex. Must be called once before any threads. */
void cbm_ui_log_init(void);

/* Append a log line to the UI ring buffer (called from log hook). */
void cbm_ui_log_append(const char *line);

Expand Down
64 changes: 62 additions & 2 deletions src/watcher/watcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "foundation/log.h"
#include "foundation/hash_table.h"
#include "foundation/compat.h"
#include "foundation/compat_thread.h"
#include "foundation/compat_fs.h"
#include "foundation/str_util.h"

Expand Down Expand Up @@ -50,6 +51,7 @@ struct cbm_watcher {
cbm_index_fn index_fn;
void *user_data;
CBMHashTable *projects; /* name → project_state_t* */
cbm_mutex_t projects_lock;
atomic_int stopped;
};
Comment on lines 51 to 56
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

projects_lock is added to protect the projects hash table, but not all accessors use it. For example, cbm_watcher_touch() and cbm_watcher_watch_count() call cbm_ht_get/cbm_ht_count without taking projects_lock, so concurrent watch/unwatch/poll can still race. Either guard every hash-table access with projects_lock (including touch/watch_count) or clearly document and enforce single-threaded callers for those APIs.

Copilot uses AI. Check for mistakes.

Expand Down Expand Up @@ -236,6 +238,7 @@ cbm_watcher_t *cbm_watcher_new(cbm_store_t *store, cbm_index_fn index_fn, void *
w->index_fn = index_fn;
w->user_data = user_data;
w->projects = cbm_ht_create(CBM_SZ_32);
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

cbm_watcher_new() doesn’t check whether cbm_ht_create() succeeded. If it returns NULL (OOM), later cbm_watcher_watch/unwatch/touch will dereference a NULL hash table via cbm_ht_get/set/delete, causing a crash. Consider handling allocation failure here (free w, destroy mutex if needed) and return NULL so callers can detect it.

Suggested change
w->projects = cbm_ht_create(CBM_SZ_32);
w->projects = cbm_ht_create(CBM_SZ_32);
if (!w->projects) {
free(w);
return NULL;
}

Copilot uses AI. Check for mistakes.
cbm_mutex_init(&w->projects_lock);
atomic_init(&w->stopped, 0);
return w;
}
Expand All @@ -244,8 +247,11 @@ void cbm_watcher_free(cbm_watcher_t *w) {
if (!w) {
return;
}
cbm_mutex_lock(&w->projects_lock);
cbm_ht_foreach(w->projects, free_state_entry, NULL);
cbm_ht_free(w->projects);
cbm_mutex_unlock(&w->projects_lock);
cbm_mutex_destroy(&w->projects_lock);
free(w);
}

Expand All @@ -264,25 +270,38 @@ void cbm_watcher_watch(cbm_watcher_t *w, const char *project_name, const char *r
}

/* Remove old entry first (key points to state's project_name) */
cbm_mutex_lock(&w->projects_lock);
project_state_t *old = cbm_ht_get(w->projects, project_name);
if (old) {
cbm_ht_delete(w->projects, project_name);
state_free(old);
}

project_state_t *s = state_new(project_name, root_path);
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

state_new() can return NULL on OOM, but the result is used immediately (s->project_name) and inserted into the hash table while holding projects_lock. Please handle allocation failure by unlocking, cleaning up, and returning (optionally logging an OOM warning).

Suggested change
project_state_t *s = state_new(project_name, root_path);
project_state_t *s = state_new(project_name, root_path);
if (!s) {
cbm_mutex_unlock(&w->projects_lock);
cbm_log_warn("watcher.watch.oom", "project", project_name, "path", root_path);
return;
}

Copilot uses AI. Check for mistakes.
if (!s) {
cbm_mutex_unlock(&w->projects_lock);
cbm_log_warn("watcher.watch.oom", "project", project_name, "path", root_path);
return;
}
cbm_ht_set(w->projects, s->project_name, s);
cbm_mutex_unlock(&w->projects_lock);
cbm_log_info("watcher.watch", "project", project_name, "path", root_path);
}

void cbm_watcher_unwatch(cbm_watcher_t *w, const char *project_name) {
if (!w || !project_name) {
return;
}
bool removed = false;
cbm_mutex_lock(&w->projects_lock);
project_state_t *s = cbm_ht_get(w->projects, project_name);
if (s) {
cbm_ht_delete(w->projects, project_name);
state_free(s);
removed = true;
}
cbm_mutex_unlock(&w->projects_lock);
if (removed) {
cbm_log_info("watcher.unwatch", "project", project_name);
}
}
Expand All @@ -291,18 +310,23 @@ void cbm_watcher_touch(cbm_watcher_t *w, const char *project_name) {
if (!w || !project_name) {
return;
}
cbm_mutex_lock(&w->projects_lock);
project_state_t *s = cbm_ht_get(w->projects, project_name);
if (s) {
/* Reset backoff — poll immediately on next cycle */
s->next_poll_ns = 0;
}
cbm_mutex_unlock(&w->projects_lock);
}

int cbm_watcher_watch_count(const cbm_watcher_t *w) {
if (!w) {
return 0;
}
return (int)cbm_ht_count(w->projects);
cbm_mutex_lock(&((cbm_watcher_t *)w)->projects_lock);
int count = (int)cbm_ht_count(w->projects);
cbm_mutex_unlock(&((cbm_watcher_t *)w)->projects_lock);
return count;
Comment on lines 322 to +329
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

cbm_watcher_watch_count() takes a const cbm_watcher_t* but then casts away const to lock/unlock projects_lock. This undermines const-correctness and can surprise callers. Consider changing the API to take a non-const cbm_watcher_t* (since it necessarily touches synchronization primitives), or store projects_lock behind an internal pointer so the public signature remains const-safe.

Copilot uses AI. Check for mistakes.
}

/* ── Single poll cycle ──────────────────────────────────────────── */
Expand Down Expand Up @@ -411,17 +435,53 @@ static void poll_project(const char *key, void *val, void *ud) {
s->next_poll_ns = ctx->now + ((int64_t)s->interval_ms * US_PER_MS);
}

/* Callback to snapshot project state pointers into an array. */
typedef struct {
project_state_t **items;
int count;
int cap;
} snapshot_ctx_t;

static void snapshot_project(const char *key, void *val, void *ud) {
(void)key;
snapshot_ctx_t *sc = ud;
if (val && sc->count < sc->cap) {
sc->items[sc->count++] = val;
}
}

int cbm_watcher_poll_once(cbm_watcher_t *w) {
if (!w) {
return 0;
}

/* Snapshot project pointers under lock, then poll without holding it.
* This keeps the critical section small — poll_project does git I/O
* and may invoke index_fn which runs the full pipeline. */
cbm_mutex_lock(&w->projects_lock);
int n = cbm_ht_count(w->projects);
if (n == 0) {
cbm_mutex_unlock(&w->projects_lock);
return 0;
}
project_state_t **snap = malloc(n * sizeof(project_state_t *));
if (!snap) {
cbm_mutex_unlock(&w->projects_lock);
return 0;
}
snapshot_ctx_t sc = {.items = snap, .count = 0, .cap = n};
cbm_ht_foreach(w->projects, snapshot_project, &sc);
cbm_mutex_unlock(&w->projects_lock);

poll_ctx_t ctx = {
.w = w,
.now = now_ns(),
.reindexed = 0,
};
cbm_ht_foreach(w->projects, poll_project, &ctx);
for (int i = 0; i < sc.count; i++) {
poll_project(NULL, snap[i], &ctx);
}
free(snap);
return ctx.reindexed;
Comment on lines 478 to 485
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The new coarse lock in cbm_watcher_poll_once() is held across the entire cbm_ht_foreach(...) call. If poll_project performs I/O, indexing work, or calls out to callbacks, this can block watch/unwatch for long periods and increase contention. A common pattern is to snapshot the current project states/keys under the lock (e.g., into a temporary array), release the lock, and then run poll_project over the snapshot to keep the critical section small.

Copilot uses AI. Check for mistakes.
}

Expand Down