Code review: unbounded Embind handle leaks pinning decompressed clusters, worker bricked on second file

AI-assisted review. Filed by agent driven by @soloturn via GDD.

Reviewed the Embind C++ bindings (libzim_bindings.cpp) and the JS glue (prejs_file_api.js/postjs_file_api.js and related files) for performance, memory consumption, simplification, and error-proneness (weighted toward the first two).

Memory (highest priority)

1. Not a single .delete() call exists anywhere in the repository — every Embind handle leaks. grep -rn "delete()" across all .js/.html/.md files returns zero hits. Embind objects are not GC'd; each is a _malloc'd C++ instance plus a registry entry that lives until .delete(). The worker glue leaks on every operation: prejs_file_api.js:20,27,31 leak entry/item/blob per article fetch (4 handles per page view); :34 — entry.getRedirectEntry()'s result is never even bound to a variable, unreachable and undeletable the instant the expression completes; :45,49 — a vector(EntryWrapper) plus one new EntryWrapper per entries.get(i) (Embind's VectorAccess::get heap-allocates a copy), so a 50-result search leaks 51 objects; :65,69 and :100,104 — same pattern for searchWithSnippets and suggest. In a worker that never restarts and calling getEntryByPath per navigation / suggest per keystroke, this leaks unboundedly. With ALLOW_MEMORY_GROWTH=1 (Makefile:173,178,183,188) the WASM heap only grows, never returns to the OS — manifests as monotonic growth to allocation failure / tab kill on mobile. Fix: wrap each handler body in try { … } finally { blob?.delete(); item?.delete(); entry?.delete(); }, delete per-index handles plus the vector in every result loop. Better: see #6.
2. A leaked BlobWrapper pins an entire decompressed ZIM cluster. libzim_bindings.cpp:32-44 — BlobWrapper stores zim::Blob m_blob by value, and zim::Blob holds a refcounted handle to the decompressed cluster buffer it points into. Leaking the wrapper (#1) leaks not ~100 bytes but the whole cluster — typically 1-4MB for zstd, shared by many articles. This also defeats libzim's own cluster cache eviction: the cache drops its reference but memory can't be reclaimed because the leaked Blob still holds one. Fix: delete the blob in the worker; independently, have getData() copy bytes out and release the Blob immediately.
3. libzim_bindings.cpp:38-40 — getContent() returns Int8Array, not the documented Uint8Array (README:90). zim::Blob::data() returns const char*, signed on wasm32, so typed_memory_view deduces the wrong element type. new Uint8Array(content) at prejs_file_api.js:30 is then a slow cross-type element-wise conversion instead of a memcpy, on the exact hot path the repo's own timing harness instruments. Fix: reinterpret_cast<const uint8_t*>(m_blob.data()), and use .slice() on the JS side.

Performance

4. prejs_file_api.js:31 — article content is structured-cloned instead of transferred, adding a third full copy (WASM heap buffer + contentArray + the clone all exist simultaneously for a 5MB article). new Uint8Array(content) already allocates a fresh, non-shared, detachable buffer — it's transferable as-is. This is exactly what the TODO on line 29 asks about. Fix: postMessage({…}, [contentArray.buffer]).
5. libzim_bindings.cpp:223,276,301 — a Searcher/SuggestionSearcher is constructed from scratch on every single query, including suggest() — the autocomplete path, called per keystroke (javascript_suggestions_usage_example.js:88) — so this pays full Xapian DB open/close per character typed. The file already has the correct long-lived pattern (SuggestionSearcherWrapper, lines 134-147) but the worker glue doesn't use it. Fix: hold file-static searcher instances alongside g_archive, reset in loadArchive.
6. prejs_file_api.js:71-77 — 5 separate WASM boundary crossings per search result (getPath/getTitle/getSnippet/getScore/getWordCount, 3 of them string marshals). At the default 50 results that's 250 crossings and 150 string marshals per search; entries.size() is also re-evaluated per loop iteration at 3 call sites. Fix: add one C++ entry point returning emscripten::val — a plain JS array of objects for the whole result set — which also eliminates #1/#7 for the search paths since a val array needs no .delete().

Error-proneness

7. Selecting a second ZIM file permanently bricks the worker. prejs_file_api.js:120 — the init handler unconditionally replaces the global Module with a bare object (Module = {}), then sets onRuntimeInitialized/preRun on it. Only works because the first init arrives before wasm instantiation finishes. On a second file selection (which tests/prototype/index.html:63 allows freely), run() never fires again, /work is never re-mounted, and every Embind function is thrown away — dead until page reload, no error surfaced. Untested by the e2e specs. Fix: guard on whether the runtime already initialized; stop reassigning Module, mutate it instead.
8. libzim_bindings.cpp:29,209,223,276,301 — g_archive dereferenced with no null check in 5 of 6 entry points (only SuggestionSearcherWrapper's constructor checks). Calling any of these before loadArchive, or after it failed (#9), traps and aborts the whole WASM instance — not caught by the surrounding try/catch since a trap isn't a C++ exception. Fix: hoist a shared null-check helper into all entry points.
9. libzim_bindings.cpp:22-25 — loadArchive has no error handling. It throws for a corrupt/non-ZIM file, called from onRuntimeInitialized (prejs_file_api.js:122), so the throw escapes the callback and the "runtime initialized" message never posts — the main thread's onmessage handler waits forever with no timeout, no error path. A user picking the wrong file gets a UI that hangs indefinitely. Fix: return a status from loadArchive, wrap the callback body in try/catch, post an error message.
10. libzim_bindings.cpp:236-239,267-270,292-295,320-323 — every search failure is silently converted to an empty result set (catch + log to std::cout + return []), despite README:187 advertising this as "proper error handling." JS can't distinguish "no fulltext index" from "zero matches" — makes the JS-side error-handling code (prejs_file_api.js:91-94) dead/unreachable, and an error UI branch (tests/prototype/index.html:115-119) that can never fire. Fix: let exceptions propagate (Embind converts them to catchable JS throws), or return an explicit status.
11. libzim_bindings.cpp:38-40 — getContent()'s returned typed array aliases HEAPU8; all four builds set -s ALLOW_MEMORY_GROWTH=1, and a memory.grow() detaches the existing ArrayBuffer. README:90 documents this as a public API with no lifetime caveat — a consumer holding the array across another Module.* call gets a silently detached, zero-length array, no exception. Fix: document the view's validity window, or return an owned copy with a separately-named zero-copy accessor.
12. libzim_bindings.cpp:274,333 + README:137-141 — searchWithLanguage's language parameter is accepted and entirely ignored (TODO at lines 282-283 admits it; body is byte-identical to search()). Separately, Embind doesn't honor C++ default arguments (a bound function() always requires full arity), so the README's documented optional language?: string signature is itself wrong. Fix: implement language selection or remove the parameter/binding and correct the README.
13. complete-whitelist.mk:21,24 — this stale duplicate patch re-introduces exactly the crash the main Makefile warns about. Its whitelist includes "el"/"hi"/"sr", which Makefile:135-136 explicitly says Xapian 1.4 has no stemmer for; it also targets libzim 9.3.0 against the Makefile's 9.8.1 and lacks the snippet-support throw patch README:21 says is required. Its own header (line 4) tells you to run it — doing so produces a build where a Greek/Hindi/Serbian ZIM aborts the module (libzim's own catch is compiled away in this configuration). Fix: delete the file; the Makefile has superseded it.
14. Makefile:173,183 vs 178,188 — -lpthread is linked on the WASM targets but not the ASM targets, though no target passes -pthread/-sUSE_PTHREADS — all four are single-threaded. The inconsistency implies a threading difference that doesn't exist, and nothing documents that these builds need no COOP/COEP headers or cross-origin isolation for consumers who must deploy the worker themselves. Fix: drop -lpthread from the WASM targets, document single-threadedness explicitly.

Simplification

• libzim_bindings.cpp:221-240 and 274-296 — searchWithLanguage is a verbatim copy of search minus a comment; collapse once #12 is resolved.
• prejs_file_api.js:126-129 — dead code building Module['arguments'] from the file list; main() (libzim_bindings.cpp:16-20) never touches argc/argv.

Minor (verified, lower impact)

• libzim_bindings.cpp:335 — register_vector<char> is registered but no bound function returns it — dead weight in the binary.
• prejs_file_api.js:23,31 — entry.isRedirect() called twice per fetch (two boundary crossings where one cached boolean would do); an unused var item = {} initializer immediately overwritten.
• prejs_file_api.js:46,66,101 — console.debug(..., entries) passes the live leaked Embind vector to devtools, retaining a strong reference and worsening #1 during the exact debugging sessions where you'd try to measure it.
• prejs_file_api_with_performance_tests.js — a drifted near-copy of prejs_file_api.js, referenced by no Makefile target, unbuildable dead code that will keep diverging.
• libzim_bindings.cpp:39 — m_blob.size() (uint64_t) narrowed to size_t (32-bit on wasm32) with no static assert or guard.
• libzim_bindings.cpp:106,109,160,177,184,201,212,215,237,268,293,315,321 — error paths log via std::cout, a line-buffered console write, on paths that can fire once per search result.

Start with libzim_bindings.cpp and prejs_file_api.js, then inspect the second-file path at tests/prototype/index.html:63 and the referenced e2e specs. Treat the review as several focused fixes rather than one change; done means each selected problem has verified behavior, including worker reinitialization, memory handling, error paths, or build consistency as applicable.