Repository navigation
fix: avoid use-after-free when re-entering ostream_redirect - #6205
Conversation
|
For completeness / JIC someone wants to pick this up later: While reviewing this PR, codex GPT-6.1-Sol ultra suggested: PR #6205 prevents the dangling-buffer failure, but does not make the manager fully nesting-aware: an inner exit still ends redirection. The added test exercises stdout with double entry and one exit; stderr and actual nested- Full nesting support would be a small, self-contained follow-up. I would make repeated entry into one manager behave like using a fresh manager at each nesting level. Currently,
That would give this the expected behavior: r = m.ostream_redirect()
with r:
write_cpp("outer-before")
with r:
write_cpp("inner")
write_cpp("outer-after") # Still redirectedA depth counter would suffice for keeping the first redirect active until the final exit. A stack gives more complete semantics: each entry captures the current There are three implementation details to handle carefully:
Tests should exercise actual nested I’d expect tens of implementation lines, with tests forming most of the follow-up. Keeping #6205 focused on preventing the dangling-buffer failure seems reasonable; nesting support establishes additional behavior that deserves its own tests and documentation. |
Description
detail::OstreamRedirect::enter()runsredirect_stdout.reset(new scoped_ostream_redirect()). Calling__enter__a second time without an__exit__in between (nesting the sameostream_redirectcontext manager) constructs the replacement beforereset()destroys the current one, so the newscoped_ostream_redirectrecordsstd::cout.rdbuf()while that still points at the buffer owned by the redirectreset()is about to delete. After the delete the live redirect holds a danglingoldstreambuf, and on__exit__its destructor restoresstd::cout/std::cerrto that freed buffer. The next write to the stream dereferences freed memory.Minimal repro, stdout only:
Releasing the existing redirect before constructing the replacement makes the new one capture the real original buffer, so no dangling pointer is left behind. Single enter/exit and sequential reuse of the same object are unaffected.
The added
test_redirect_reentrysegfaults on the current headers and passes with the fix.This fix was found and prepared with AI assistance and reviewed by me before submitting.
Suggested changelog entry:
py::add_ostream_redirectwhen the context manager's__enter__was entered again without an intervening__exit__.