Skip to content

fix: To fix decoder lifetime during async decoding#711

Open
kaiqiy-nv wants to merge 1 commit into
NVIDIA:mainfrom
kaiqiy-nv:kaiqiy/fix-decoder-lifetime-async
Open

fix: To fix decoder lifetime during async decoding#711
kaiqiy-nv wants to merge 1 commit into
NVIDIA:mainfrom
kaiqiy-nv:kaiqiy/fix-decoder-lifetime-async

Conversation

@kaiqiy-nv

Copy link
Copy Markdown
Collaborator

This PR tries to fix [B] 6479862.
It also adds 1 C++ test and 1 Python test to expose the bug and verify the fix.
Thanks for looking at this PR.

In this suggested fix:
[1] We keep the decoder alive for the full asynchronous operation.
We make decoder support shared_from_this(), return shared_ptr from public decoder factories, and capture shared ownership in decode_async instead of raw this.
Plugin creators keep unique_ptr and convert ownership only at the factory boundary.
[2] We update decoder storage and Python bindings to preserve shared ownership.
This avoids use-after-free when callers release a decoder before retrieving its asynchronous result.

We also add two test cases to expose this bug and verify the fix:

  • DecoderAsyncLifetime.PendingAsyncKeepsDecoderAlive: We block an instrumented decoder inside decode(), release the caller’s decoder handle, and check that the destructor has not run. We then unblock the worker and verify its result.
  • test_decode_async_temporary_keeps_decoder_alive: We call decode_async on a temporary Python decoder and verify that .get() returns a valid result, proving that the temporary remains alive until decoding completes.

Signed-off-by: Kaiqi Yan <kaiqiy@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@bmhowe23 bmhowe23 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @kaiqiy-nv. We are close to a code freeze. Since this changes interfaces and the problematic use case appears to be not a primary use case, I'd like to hold off on this one for at least this week. Feel free to poke us about this in an about a week, though. Thanks!

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