diff --git a/TODOS.md b/TODOS.md index 445c5f6..eed6ab9 100644 --- a/TODOS.md +++ b/TODOS.md @@ -12,6 +12,26 @@ parsers via a pool or per-thread storage, plugged in behind `open_parser` without touching call sites. A `TSParser`/`TSQueryCursor` is one-per-thread. - Full write-up: `GRAMMAR_CACHE_THREADING.md`. +- [ ] Restore parallel grammar preload (BEHAVIOUR CHANGE in `edb2e64`). + `preload_grammars` used to `dlopen`+compile each grammar on its own thread; + it is now serial (`grammar()` per lang on the calling thread). The startup + dlopen cost is no longer overlapped. + - Why serial for now: `grammar()` holds `registry.mu` across the whole + dlopen+compile, so naively fanning out `grammar()` calls just queues every + worker on that one lock — zero real parallelism. + - To bring it back: move `build_grammar` (dlopen + `compile_query`, no shared + mutable state) OUTSIDE the lock; take `mu` only to (1) init the registry + + snapshot which langs are missing, and (2) briefly publish each finished + `^Grammar`. Restore the `thread`-based worker fan-out + `SPALL` + init/cleanup hooks that `edb2e64` deleted. Safe because glibc `dlopen`, the + heap allocator, and each thread's own temp allocator are all thread-safe. + - An `RW_Mutex` does NOT help the parallel-load side (writers stay exclusive); + it only helps a future multi-threaded *read* path (concurrent cache-hit + `grammar()` lookups during a parallel render). Consider it only if/when + render itself goes multi-threaded — then pair rlock hits with off-lock build + + a double-check-and-discard on the miss race (needs a `free_grammar`). + - Assumption to preserve: preload completes before any concurrent `grammar()` + use (render is single-threaded and runs after preload). - [ ] instead of `warnings: [dynamic]Error` we should use `warnings: [dynamic; 8]Error` - when limit reached, the template fails and stops rendering. - [ ] Brainstorm plugin/tool/(postccs/less/tailwind/sass) support. diff --git a/markdown/highlight.odin b/markdown/highlight.odin index 0ef5499..bebaa06 100644 --- a/markdown/highlight.odin +++ b/markdown/highlight.odin @@ -90,9 +90,12 @@ write_escaped :: proc(b: ^strings.Builder, s: string) { } } +// unescape_html returns a caller-owned string (free it with delete). When the +// input has no entities it returns a clone; otherwise it returns the builder's +// buffer. It must NOT destroy the builder — to_string aliases that buffer, so the +// returned string owns it. unescape_html :: proc(s: string) -> string { sb := strings.builder_make() - defer strings.builder_destroy(&sb) start := 0 for i in 0 ..< len(s) { @@ -119,7 +122,7 @@ unescape_html :: proc(s: string) -> string { strings.write_string(&sb, replacement) start = i + semi + 1 } - if start == 0 do return s + if start == 0 do return strings.clone(s) if start < len(s) do strings.write_string(&sb, s[start:]) return strings.to_string(sb) } @@ -136,6 +139,7 @@ highlight_block :: proc(code: string, lang: string, file_path: string) -> string defer ts.parser_delete(parser) raw_code := unescape_html(code) + defer delete(raw_code) raw_c := strings.clone_to_cstring(raw_code) defer delete(raw_c) diff --git a/markdown/highlight_test.odin b/markdown/highlight_test.odin index a37c7ad..87c0f34 100644 --- a/markdown/highlight_test.odin +++ b/markdown/highlight_test.odin @@ -81,3 +81,23 @@ test_highlight_code_wraps_css_block :: proc(t: ^testing.T) { "block body should be highlighted", ) } + +// unescape_html must return a caller-owned allocation the caller frees exactly +// once. The buggy version frees its builder internally (defer builder_destroy) +// and returns the freed alias, so the caller's delete becomes a double-free that +// the test runner's tracking allocator flags. +@(test) +test_unescape_html_entities_owned :: proc(t: ^testing.T) { + got := unescape_html("<a>") + defer delete(got) + testing.expect_value(t, got, "") +} + +// Same contract on the no-entity path: the buggy version returns the borrowed +// input, so freeing it is a bad free (pointer not from the allocator). +@(test) +test_unescape_html_passthrough_owned :: proc(t: ^testing.T) { + got := unescape_html("plain text") + defer delete(got) + testing.expect_value(t, got, "plain text") +}