NVIDIA / NVIDIA/TensorRT-Model-Connect
Cross-family runtime safety and long-context issues found during #1158 review
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 254
- Forks
- 58
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 235
Description
Context
Reviewing #1158 surfaced several defects in code copied from the existing Llama/model-family templates. I re-audited each call path against current main and kept only findings with a concrete failure mechanism. These are not SmolLM3-specific regressions and should not be fixed inside #1158.
#1176 already tracks two other confirmed shared defects: unchecked CUDA device queries in the sparse multinomial policy and GPU fixture tests that can report a false pass.
The counts below are occurrences of the exact code pattern on main, not a claim that every family enables the affected optional feature.
Confirmed findings
-
Check sampler
cudaMallocresults transactionally — allocations are used without checking status. Inensure_device_buffers,capacity_ = keepruns unconditionally after bothcudaMalloccalls, so a failed allocation still advances the capacity and is never retried: every later call withkeep <= capacity_skips reallocation and reuses the bad pointers. The sparse path can then return a stale token whend_token_id_is null, and the greedy CUDA kernel can receive a null output pointer. Add allocation-failure injection coverage, throw on allocation failure, and only commit new pointers and capacity after every allocation succeeds. Original finding: https://github.com/NVIDIA/TensorRT-Model-Connect/pull/1158#discussion_r3938042604 -
Harden temporary FFI kernel loading — bundle kernel bytes are written with
std::ofstreamto predictable/tmp/trtmc_kernel_<global_name>.sopaths. Creation is neither exclusive nor no-follow, the complete write is not checked, and files are not removed. A pre-created symlink is followed and concurrent processes using the same global name share the same file before it is loaded. The exact filename pattern appears in 83 family helper files.This one does not need a new design.
src/runtime/models/qwen3_8/plugin_helpers.cppalready carries a hardened version of the same helper: it creates a private0700directory withmkdtemp, opens the file withO_CREAT | O_EXCL | O_NOFOLLOWandS_IRUSR | S_IWUSR, loops until the whole buffer is written, checksclose, cleans up on any failure, and pairs with aremove_kernel_so_tempthat unlinks the file and directory after the loader is done. Its comment states the same threat model. The work is to propagate that implementation to the remaining 83 files, which keeps the change mechanical and gives the reviewer an in-repo reference to compare against rather than a new design to judge. Original finding: https://github.com/NVIDIA/TensorRT-Model-Connect/pull/1158#discussion_r3938042595 -
Keep TriAttention's scored row count out of CUDA
grid.y—launch_score_kernelpassescandidate_countasgrid.yand the kernel reads it throughblockIdx.y, and the caller passestotal_tokens, the current cached row count. CUDA's y-dimension limit is 65,535, so a cache of 65,536 rows makes the launch fail;cudaGetLastError()then returnskFailed,run_gpu_selection_over_layersreturns false, and selection falls back to the host implementation with no diagnostic. Swap the grid dimensions or chunk the launch, and test the 65,535/65,536 boundary.On reachability, so this is not read as broader than it is: TriAttention is a per-bundle opt-in (
tri_cfg.enabledfrom..._parse_triattention_bundle_config, which also requires a stats section in the bundle), no family declares it inMODEL.toml, and the GPU selection path only runs oncetotal_tokensexceeds the keep budget. Reaching the limit therefore needs a bundle that enables TriAttention with a budget near 65,536, which is an unusual configuration for a cache-compaction strategy. The boundary itself is deterministic once reached. Original finding: https://github.com/NVIDIA/TensorRT-Model-Connect/pull/1158#discussion_r3938042609
Suggested implementation shape
Use this issue as a tracker, but fix each category in its own mechanical cross-family PR. Each PR should first add a focused regression test or deterministic failure injection, then update every family carrying the same reachable implementation.
Potential findings whose impact or intended policy is not yet proven are deliberately excluded. In particular, this issue does not claim a correctness race from the sampler's legacy default stream, does not propose a rank-1 mask sweep without a reachable current engine, and does not prescribe the intended E2E prefix-comparison policy.
An earlier revision of this issue listed the unbounded linear-spec masked-token loop as a fourth finding. I have removed it: supports_text_diffusion defaults to false in every family and only nemotron_labs_diffusion sets it, so the 26 other pipelines carrying that loop throw in resolve_text_diffusion_block_length before reaching it. The remaining exposure is one family under one runtime strategy, which is too narrow to track here.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/runtime/models/qwen3_8/plugin_helpers.cpp as the in-repository reference, then locate ensure_device_buffers, launch_score_kernel, run_gpu_selection_over_layers, and the 83 family helper files carrying the temporary-loader pattern. Add focused allocation-failure and 65,535/65,536 boundary coverage, then update each affected family. Done means failures are handled safely, temporary files are cleaned up, and the CUDA boundary no longer causes silent fallback.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, security, testing
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100