Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 3, 2025

Fix memory leak by making NativeFinalizer instances static

The issue is that NativeFinalizer instances were being created as instance variables instead of static variables. According to Dart FFI best practices, finalizers should be static and shared across all instances of a class for proper memory management.

Root Cause

Creating a new NativeFinalizer for each instance is inefficient and can prevent proper finalization:

  • Performance overhead from creating multiple finalizers
  • Dart runtime may not efficiently manage finalization
  • Memory accumulates because finalizers don't trigger properly

Solution

  • Analyze the code and identify the root cause
  • Convert all NativeFinalizer instance variables to static variables in:
    • Tree class (line 125)
    • Parser class (line 61)
    • CancelToken class (line 113)
    • TreeCursor class (line 145)
    • Query class (line 156)
    • QueryCursor class (line 246)
  • Fixed bug in QueryCursor: was using ts_query_delete instead of ts_query_cursor_delete
  • Verified all changes are syntactically correct
  • Updated CHANGELOG.md with version 0.2.1
  • Bumped version in pubspec.yaml to 0.2.1

Expected Result

Memory usage when parsing 50k files should now:

  • Peak at ~1.3 GB (down from 15 GB)
  • Match the manual deletion behavior
  • Automatically free memory as Tree objects go out of scope

The fix ensures one NativeFinalizer per class type, allowing efficient memory management by the Dart runtime.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: TimWhiting <15617545+TimWhiting@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] The Tree Finalizer appears to not be working. Fix memory leak by making NativeFinalizer instances static Oct 3, 2025
@Copilot Copilot AI requested a review from TimWhiting October 3, 2025 22:57
Copilot finished work on behalf of TimWhiting October 3, 2025 22:57
@TimWhiting
Copy link
Collaborator

@copilot
I don't know that this is the actual root cause of the user's issue, but it seems like a step in the right direction. I've removed the Fixes PR# description. Please update the changelog, and bump the version.

Co-authored-by: TimWhiting <15617545+TimWhiting@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 3, 2025

@copilot
I don't know that this is the actual root cause of the user's issue, but it seems like a step in the right direction. I've removed the Fixes PR# description. Please update the changelog, an...

Updated the changelog and bumped version to 0.2.1 in commit 1b26f4f.

Copilot finished work on behalf of TimWhiting October 3, 2025 23:03
@TimWhiting TimWhiting marked this pull request as ready for review October 4, 2025 02:36
@TimWhiting TimWhiting merged commit ebc397f into master Oct 4, 2025
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.

2 participants