Skip to content

fls: fix a real crash and a goto-definition correctness/perf bug - #204

Merged
omdxp merged 10 commits into
mainfrom
fls-goto-hover-bugs
Oct 14, 2026
Merged

omdxp merged 10 commits into
mainfrom
fls-goto-hover-bugs

Conversation

@omdxp

@omdxp omdxp commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • The real bug behind the actual symptoms: editing any one open document corrupted hover/go-to-definition in any other open document that happened to share an import. A shared import's cache entry got freed and replaced whenever a document with a different import graph was analyzed, even when that imported file's own text never changed (the cache's staleness check is keyed to the whole preload-discovered symbol set, not per-file content) - every other document's own already-merged program still borrowed strings from that entry's now-freed tokens. Confirmed directly with a 20-round multi-file edit/hover/goto stress test: corrupted on the very first edit before the fix, clean 20/20 after. Left to leak instead of free, since nothing in this cache's current design can yet prove when the last borrower of an old revision is gone.
  • Fixed a real SIGSEGV crash: _free_fn_sig_map freed each method-table key while still probing the same live map for every other key. Open-addressing can walk through an earlier key's now-freed (but never tombstoned) slot on a hash collision, and the later lookup's strcmp against that dangling pointer is what was crashing. Split into two passes (free every value, then every key), matching the pattern already used correctly elsewhere in this file.
  • Fixed a goto-definition performance bug: answer_definition was unconditionally rebuilding the entire workspace symbol table before even trying the cheap same-file-plus-imports lookup, on every single request. Now it only pays for the one-time workspace file-discovery walk; the expensive rebuild only runs on the genuine fallback path. First call: ~1s one-time cost. Every call after: instant.

Test plan

  • 20-round multi-file stress test (edit one document, hover/goto-def in a different, unedited one that shares an import): corrupted every round before the cache fix, clean every round after
  • Minimal 9-line repro isolating the crash, 5/5 clean under Guard Malloc (default 16-byte and strict 8-byte pointer-size alignment)
  • fun test . x2 clean (56/56 files)
  • 3-stage bootstrap, stage-3 installed and re-verified
  • Crash-safety probes (syntax error, type error + follow-up edit): no crash
  • Repeated hover + goto-definition cycles on the originally reported symbol: consistent, correct, instant after the first call
  • Goto-definition + hover across several different symbols: all resolve to correct locations with matching hover content

omdxp added 10 commits October 14, 2026 01:12
Stored MethodTable by pointer instead of by value; also freed a duplicate method-name key's own fresh copy instead of leaking it.
…okup

A later key's own hash probe can walk through an earlier key's slot on a collision; freeing each key before every lookup ran left that walk reading freed memory.
Only its own slow-path fallback ever needs the full table; every other call now only pays the one-time workspace file-discovery walk.
A shared import's cache entry got freed and replaced whenever any document with a different import graph was analyzed, even when that file's own text never changed - every other document's own merged program still borrowed strings from its freed tokens. Left to leak instead.
The setting set FLS_DEBUG_IMPORTS in the spawned process's own env, but nothing in fls ever read it; the real import-resolution error was always discarded in favor of a generic message.
Hundreds of fork/join cycles back to back stressed the scheduler; traced a real SIGABRT + corrupted import path to it.
pthread_create defaults joinable; the scheduler's elastic workers, base pool, and the deadlock watchdogs spawn and exit without ever being joined, leaking the handle (and stack reservation on glibc) every contention burst. Detach each right after a successful spawn.
A second edit landing before the first was analyzed silently overwrote has_edit_range's own range, discarding the first entirely.
A cloned Import node aliased the same owns_path string as the node it was cloned from. Whichever side's teardown ran first freed it out from under the other - the real cause of the goto/hover corruption.
Only a plain name() call's return type was resolved; name.method() fell through silently, so removing a method-call fit's own catch-all branch never warned.
@omdxp
omdxp merged commit a983ae0 into main Oct 14, 2026
4 checks passed
@omdxp
omdxp deleted the fls-goto-hover-bugs branch October 14, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant