Skip to content

Comments

Implement AutoCloseable for Native Resources as an Optional Route for Deterministic Memory Collection#129

Open
DavidBakerEffendi wants to merge 12 commits intobonede:mainfrom
DavidBakerEffendi:dave/autocloseable
Open

Implement AutoCloseable for Native Resources as an Optional Route for Deterministic Memory Collection#129
DavidBakerEffendi wants to merge 12 commits intobonede:mainfrom
DavidBakerEffendi:dave/autocloseable

Conversation

@DavidBakerEffendi
Copy link

This PR updates the Java bindings to implement the AutoCloseable interface for all classes wrapping native C structures (e.g., TSParser, TSQuery, TSTree). This change allows these resources to be used within try-with-resources blocks, ensuring deterministic cleanup of native memory and preventing resource leaks. It also integrates java.lang.ref.Cleaner as a safety net for garbage collection and implements strict state checking to preventing "use-after-free" segfaults.

Key changes

  • AutoCloseable Implementation: TSParser, TSLanguage, TSQuery, TSQueryCursor, TSTree, TSTreeCursor, and TsLookAheadIterator now implement AutoCloseable.
  • Deterministic Cleanup: The close() method invokes a registered Cleanable action to immediately free native pointers.
  • Fail-Fast Protection: Public methods in these classes now call ensureOpen(), throwing an IllegalStateException if accessed after being closed, preventing JVM crashes.
  • Cleaner Integration: CleanerRunner.register now returns a Cleanable instance, allowing manual invocation while maintaining GC-based safety.
  • Code Cleanup: Imported Cleaner.Cleanable to reduce verbosity and added JavaDoc notes explaining testing limitations for classes like TSTree.
  • Testing: Added TSAutoCloseableTest to verify auto-close behavior and idempotency.

Resolves #110

bonede pushed a commit that referenced this pull request Feb 24, 2026
…after-free / double-free

PR #129 introduced AutoCloseable on all tree-sitter wrapper classes using
Cleaner.Cleanable (idempotent, so close()+GC never double-frees). However
TSTree and TSLanguage were missing the `closed` boolean + ensureOpen()
pattern that the other classes (TSParser, TSTreeCursor, TSQuery, etc.) use.

Without it:
- TSTree.copy() on a closed tree calls ts_tree_copy(freed_ptr), returns an
  undefined pointer, and registers a new Cleanable for it.  If that pointer
  happens to alias the original, the GC-triggered cleaner on the copy would
  call ts_tree_delete a second time → double-free.
- All other TSTree public methods (getRootNode, edit, getIncludedRanges,
  getChangedRanges, printDotGraphs) silently use a freed native pointer.
- TSLanguage has the same gap: no closed guard means all public methods and
  getPtr() can operate on a freed language pointer, which affects TSParser,
  TSQuery, and TsLookAheadIterator that hold references to the language.

Fix:
- TSTree: add `closed` boolean, ensureOpen(), set closed=true in close(),
  and add ensureOpen() to all public methods and getPtr() (the latter
  automatically protects TSParser's oldTree.getPtr() calls too).
- TSLanguage: same pattern – add closed/ensureOpen() and guard getPtr(),
  copyPtr(), nextState(), stateCount().

https://claude.ai/code/session_01GWFZFAWaU9eTj8RmRKcB9m
@DavidBakerEffendi
Copy link
Author

Noted the review. Will address that soon

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.

In a concurrent scenario, we need deterministic resource release!

1 participant