| Author | ![[]](/git/avatar/c50933ca2aa61e0fe2c43d46bb6b59cb_avatar) Andy Green <andy@warmcat.com> 2026-09-06 06:44 UTC | | Committer | Andy Green <andy@warmcat.com> 2026-09-06 06:44 UTC | | Tree | 5bf6c9e6ad8e52cdbc0839ee83fd892694900bcb ![]() Raw Patch | | | ![]() web: close artifact db blob and drop cache refcount after /artifacts/ GET, fixes F-003 | web: close artifact db blob and drop cache refcount after /artifacts/ GET, fixes F-003
saiw_get_blob() opens a read-only sqlite3 blob on the event db and takes a
refcount on the cached db handle, storing both on pss for the HTTP writeable
path to stream from. But nothing ever released them: the writeable loop
simply stopped when artifact_offset reached artifact_length, and the only
cleanup (LWS_CALLBACK_CLOSED) is a ws-role reason that an HTTP transaction
never sees. Every /artifacts/<task_uuid>/<down_nonce>/... GET therefore
leaked one sqlite3_blob handle plus one db cache refcount, unauthenticated
(the 32-hex down_nonce capability is rendered into the public build page for
every viewer), and unbounded under sustained requests. An open blob also
holds a read transaction on the event db, so each leak additionally pins
that db's WAL against checkpointing while build logs keep being appended.
Release the artifact state at all three places the stream can end:
- HTTP_WRITEABLE, when the final part went out: close the blob and drop
the db refcount immediately, so the handle and the read transaction live
exactly as long as the active download;
- CLOSED_HTTP: the client went away mid-stream (or the connection closed
after a completed transfer), the wsi is going away;
- HTTP_DROP_PROTOCOL: the transaction is being unbound from this protocol;
on a keepalive connection moving on to its next transaction this is the
last look at the old pss before lws frees and reallocated it, which is
exactly where an interrupted transfer's handles would otherwise be lost.
The cleanup mirrors the sai-server builder-side handling of the same
pss->blob_artifact / pss->pdb_artifact pair in s-comms.c.
|
diff --git a/src/web/w-comms.c b/src/web/w-comms.c
index 9fd99d2..c378d56 100644
--- a/src/web/w-comms.c
+++ b/src/web/w-comms.c
@@ -132,6 +132,30 @@ saiw_event_db_close_all_now(struct vhd *vhd)
return 0;
}
+/*
+ * Release any artifact-download state on pss. saiw_get_blob() opened a
+ * read-only blob on the event db and took a refcount on the cached db handle;
+ * the open blob also pins a read transaction on that db (blocking WAL
+ * checkpointing while it exists). Once the artifact has gone out -- or the
+ * client went away mid-stream, or the transaction is being unbound from us on
+ * a keepalive connection that is moving on to a fresh transaction with a
+ * fresh pss -- the blob and the db refcount must be released here.
+ */
+static void
+saiw_close_artifact(struct pss *pss)
+{
+ if (pss->blob_artifact) {
+ sqlite3_blob_close(pss->blob_artifact);
+ pss->blob_artifact = NULL;
+ }
+
+ if (pss->pdb_artifact) {
+ sai_event_db_close(&pss->vhd->sqlite3_cache,
+ &pss->pdb_artifact);
+ pss->pdb_artifact = NULL;
+ }
+}
+
static int
w_callback_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
void *in, size_t len)
@@ -397,7 +421,22 @@ http_resp:
if (pss->artifact_offset != pss->artifact_length)
lws_callback_on_writable(wsi);
+ else
+ /* the last part went out, we're done with the blob */
+ saiw_close_artifact(pss);
+
+ break;
+
+ case LWS_CALLBACK_CLOSED_HTTP:
+ /* the http conn went away, eg, mid-artifact-download */
+
+ saiw_close_artifact(pss);
+ break;
+
+ case LWS_CALLBACK_HTTP_DROP_PROTOCOL:
+ /* the transaction is unbinding from us, drop artifact state */
+ saiw_close_artifact(pss);
break;
/*
|