ghostty_string_s is returned by three exports with two different ownership
rules, and nothing on the managed side can tell them apart.
ghostty_config_open_path and ghostty_surface_tty_name return heap
memory, freed with ghostty_string_free.
ghostty_config_builtin_theme returns static storage and must never be
passed to it. Freeing it hands the allocator a pointer it never owned:
an invalid-free assert in debug, heap corruption in release.
Today the only thing preventing that is a comment. Both the header and the
managed import now carry the warning, which is enough to be correct but not
enough to be safe: the next person adding a caller reads the type, not the
comment on one of its producers.
Second half: nothing frees the ones that should be freed
ghostty_string_free is not bound anywhere in windows/. So
ghostty_config_open_path's result leaks on every call. It is small and
once per config load, which is why it has gone unnoticed, but it means the
managed side has never exercised the owning half of this contract at all.
Suggested shape
Make the distinction structural rather than documented. Options, roughly in
order of preference:
- Two managed types over the same native struct -- an owning
GhosttyOwnedString that is IDisposable and calls ghostty_string_free,
and a borrowed GhosttyStaticString that cannot. The native side is
unchanged; the compiler enforces the rule at every call site.
- Return the static one as
const char* plus a length, so the type itself
says it is not the owned thing.
- Dupe the built-in theme into the global allocator so all three obey one
rule, and bind ghostty_string_free -- which also fixes the leak.
Whichever is chosen, bind ghostty_string_free and use it for the two
owning exports.
ghostty_string_sis returned by three exports with two different ownershiprules, and nothing on the managed side can tell them apart.
ghostty_config_open_pathandghostty_surface_tty_namereturn heapmemory, freed with
ghostty_string_free.ghostty_config_builtin_themereturns static storage and must never bepassed to it. Freeing it hands the allocator a pointer it never owned:
an invalid-free assert in debug, heap corruption in release.
Today the only thing preventing that is a comment. Both the header and the
managed import now carry the warning, which is enough to be correct but not
enough to be safe: the next person adding a caller reads the type, not the
comment on one of its producers.
Second half: nothing frees the ones that should be freed
ghostty_string_freeis not bound anywhere inwindows/. Soghostty_config_open_path's result leaks on every call. It is small andonce per config load, which is why it has gone unnoticed, but it means the
managed side has never exercised the owning half of this contract at all.
Suggested shape
Make the distinction structural rather than documented. Options, roughly in
order of preference:
GhosttyOwnedStringthat isIDisposableand callsghostty_string_free,and a borrowed
GhosttyStaticStringthat cannot. The native side isunchanged; the compiler enforces the rule at every call site.
const char*plus a length, so the type itselfsays it is not the owned thing.
rule, and bind
ghostty_string_free-- which also fixes the leak.Whichever is chosen, bind
ghostty_string_freeand use it for the twoowning exports.