Skip to content

[tfjs-node] Prevent tensor ID rollover leaks and collisions - #8739

Open
GideonKoenig wants to merge 1 commit into
tensorflow:masterfrom
GideonKoenig:fix-tfjs-node-tensor-id-rollover
Open

[tfjs-node] Prevent tensor ID rollover leaks and collisions#8739
GideonKoenig wants to merge 1 commit into
tensorflow:masterfrom
GideonKoenig:fix-tfjs-node-tensor-id-rollover

Conversation

@GideonKoenig

Copy link
Copy Markdown

Fixes #8735.

Summary

tfjs-node assigns native tensor handles signed 32-bit IDs. When the counter reaches INT32_MAX, the current implementation relies on signed overflow and begins returning negative IDs.

This causes two problems:

  1. NodeJSKernelBackend.disposeData() only deletes IDs greater than or equal to zero, so native handles allocated after rollover are leaked.
  2. After the counter traverses the full ID range, a candidate ID may already belong to a persistent tensor. The current unordered_map::insert result is returned without checking whether insertion succeeded, which can alias the new JavaScript tensor to the persistent tensor's native handle.

Disposing the temporary tensor can then delete the persistent tensor's handle.

Changes

This PR:

  • advances tensor IDs with an explicit INT32_MAX to INT32_MIN transition, avoiding signed-overflow behavior;
  • reserves -1, which is already used to represent tensor data that has not been uploaded to the native backend;
  • allows disposeData() to delete negative native tensor IDs while continuing to exclude the -1 sentinel;
  • checks whether tfe_handle_map_.emplace() successfully inserted the new handle;
  • retries with subsequent IDs when a candidate is already owned by a live tensor;
  • adds an internal setNextTensorIdForTest() binding hook for deterministic rollover and collision coverage. This is not added to the public TFJSBinding TypeScript interface.

In the normal allocation path, emplace() performs the same map lookup needed for insertion. Additional attempts occur only when an occupied ID is encountered.

Testing

Added coverage for:

  • INT32_MAX rolling over to INT32_MIN;
  • skipping the reserved -1 ID;
  • avoiding a live persistent tensor's ID;
  • deleting a temporary tensor without affecting the persistent tensor;
  • disposing native tensors with negative IDs through the real Node backend;
  • returning the native tensor count to its original value after cleanup.

Local validation under Node.js 20.20.2:

  • native addon source build passed;
  • TypeScript build passed;
  • focused rollover tests: 2 specs, 0 failures;
  • complete tfjs-node suite: 4,045 specs, 0 failures, 5 existing pending specs;
  • tfjs-node lint passed;
  • repository root lint passed.

@google-cla

google-cla Bot commented Jul 27, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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.

[tfjs-node] Native tensor handles leak after signed int32 tensor ID rollover

1 participant