Skip to content

runtime: flush stdout and stderr file descriptors at exit - #38

Merged
gf712 merged 8 commits into
mainfrom
fix-buffer-flush
Jul 28, 2026
Merged

runtime: flush stdout and stderr file descriptors at exit#38
gf712 merged 8 commits into
mainfrom
fix-buffer-flush

Conversation

@gf712

@gf712 gf712 commented Jul 27, 2026

Copy link
Copy Markdown
Owner

No description provided.

Comment thread src/executable/bytecode/BytecodeProgram.cpp Outdated
Comment thread src/runtime/modules/IOModule.cpp
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 2 new issues — see the inline comments.

Comment thread src/executable/bytecode/BytecodeProgram.cpp
Comment thread src/runtime/modules/IOModule.cpp
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 2 new issues — see the inline comments.

@gf712
gf712 force-pushed the fix-buffer-flush branch from 0b2dff6 to efcfb85 Compare July 27, 2026 12:21
Comment thread src/runtime/modules/IOModule.cpp
Comment thread src/runtime/modules/IOModule.cpp Outdated
Comment thread src/runtime/modules/IOModule.cpp
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 3 new issues — see the inline comments.

@gf712
gf712 force-pushed the fix-buffer-flush branch from efcfb85 to 306cd11 Compare July 27, 2026 13:45
Comment thread integration/run_python_tests.sh
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 1 new issue — see the inline comment.

@gf712
gf712 force-pushed the fix-buffer-flush branch from 306cd11 to 81b0d4f Compare July 27, 2026 14:06
Comment thread src/runtime/modules/IOModule.cpp Outdated
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 1 new issue — see the inline comments.

Comment on lines +3334 to +3339
PyResult<PyObject *> isatty()
{
return m_buffer->get_method(PyString::create("isatty").unwrap()).and_then([](auto *isatty) {
return isatty->call(nullptr, nullptr);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

isatty() dereferences m_buffer with no initialization check (bug)

m_buffer is only set in init(), and TextIOWrapper.__new__ allocates without initializing it, so _io.TextIOWrapper.__new__(_io.TextIOWrapper).isatty() (or type(sys.stdout).__new__(type(sys.stdout)).isatty()) null-derefs and crashes. Previously isatty() resolved to the inherited IOBase::isatty, which safely returns False after a check_closed() — so this is a new crash path on a call that was previously safe, not just a pre-existing hazard. BufferedWriter in this same PR guards the analogous case via check_initialized().

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Code review

Found 1 new issue — see the inline comments.

@gf712
gf712 force-pushed the fix-buffer-flush branch from b9c384b to e97e80c Compare July 28, 2026 19:24
Comment on lines +536 to +543
PyResult<std::monostate> Interpreter::finalise()
{
// TODO: capture exceptions, and raise last one
for (auto [callback, args] : m_callbacks) {
[[maybe_unused]] auto result = callback->call(args, nullptr);
}

return Ok(std::monostate{});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

finalise() discards the result of every exit callback ([[maybe_unused]] auto result = ...) and always returns Ok. The caller in BytecodeProgram.cpp also discards finalise()'s return value, so the process exit code depends only on the main function's result. Since this same PR makes print() default to flush=False and ties sys.stdout line-buffering to isatty(), a flush failure during exit (e.g. EPIPE on a closed pipe, ENOSPC) is now silently swallowed instead of surfacing as a Python error — output can be silently truncated while the process still exits 0. (bug)

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown

Code review

Found 1 new issue — see the inline comment.

@gf712
gf712 merged commit 74079d9 into main Jul 28, 2026
4 checks passed
@gf712
gf712 deleted the fix-buffer-flush branch July 28, 2026 19:42
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.

1 participant