Possible progress callback leak in `Parser.parse()`

[OvOhao]

Possible progress callback leak in Parser.parse()

I found a possible native and JavaScript reference leak when Parser.parse() is called with progressCallback.

File: src/parser.cc

Functions: CallbackProgress::Make, Parser::Parse

Relevant JavaScript API:

Parser.prototype.parse = function(input, oldTree, {
  bufferSize,
  includedRanges,
  progressCallback
} = {}) {
  const tree = parse.call(
    this,
    input,
    oldTree,
    bufferSize,
    includedRanges,
    progressCallback,
  );
  // ...
}

The native wrapper allocates a callback payload and stores a persistent function
reference:

class CallbackProgress final {
 public:
  static TSParseOptions Make(const Napi::Function &func) {
    TSParseOptions options;
    auto *callback = new CallbackProgress();
    callback->func = Napi::Persistent(func);
    options.payload = static_cast<void *>(callback);
    options.progress_callback = Cancel;
    return options;
  }

 private:
  Napi::FunctionReference func;
  // ...
};

Parser::Parse() passes the options to Tree-sitter:

if (info.Length() > 4 && info[4].IsFunction()) {
  TSParseOptions options = CallbackProgress::Make(info[4].As<Function>());
  tree = ts_parser_parse_with_options(parser_, old_tree, callback_input.Input(), options);
} else {
  tree = ts_parser_parse(parser_, old_tree, callback_input.Input());
}
return Tree::NewInstance(env, tree);

There is no matching delete or func.Reset() after
ts_parser_parse_with_options() returns. TSParseOptions is stack-local, and
Tree-sitter only receives the raw payload pointer for the parse operation.

For comparison, logger payload ownership is handled explicitly:

if (current_logger.payload != nullptr) {
  delete static_cast<Logger *>(current_logger.payload);
}

Suggested fix: keep the progress callback payload owned by Parser::Parse()
and release it after ts_parser_parse_with_options() returns, for example by
using a stack object or std::unique_ptr<CallbackProgress>.

[michaelgiraldo]

Thanks for documenting this. We independently reproduced the parser-side retention in a downstream project using tree-sitter@0.25.1, Node 26.5.1, and macOS arm64.

In a forced-GC probe with 100 distinct progressCallback closures passed to Parser.parse(), all 100 callbacks remained reachable after the parses completed and application references were dropped. Control callbacks that were not passed to the parser were collected. That matches the ownership path described here: CallbackProgress::Make() allocates with new, the persistent N-API function reference lives in that allocation, and Parser::Parse() has no matching owner or release after ts_parser_parse_with_options() returns.

We also noticed the analogous unowned allocation in src/query.cc. The current public JavaScript query path appears inconsistent, though: matches() does not forward progressCallback, while captures() forwards it but the native Captures path currently executes without options. Our public-API probe therefore did not invoke or retain query progress callbacks. The parser leak is directly reachable; the query allocation should still be given explicit ownership before that wiring is enabled or corrected.

A stack- or RAII-owned callback holder scoped across the synchronous native operation appears sufficient for Parser::Parse(). The query holder would need to remain alive through the synchronous match/capture iteration and then be released before returning.

[martinezcarlos3316-eng]