From 156332c90e7bafd5c288ce4c162e35a5226000fe Mon Sep 17 00:00:00 2001 From: Agisilaos Kounelis Date: Thu, 9 Apr 2026 02:42:03 +0300 Subject: [PATCH 1/6] fix: segfault when moving `scoped_ostream_redirect` The default move constructor left the stream (`std::cout`) pointing at the moved-from `pythonbuf`, whose internal buffer and streambuf pointers were nulled by the move. Any subsequent write through the stream dereferenced null, causing a segfault. Replace `= default` with an explicit move constructor that re-points the stream to the new buffer and disarms the moved-from destructor. --- include/pybind11/iostream.h | 12 ++++++++++-- tests/test_iostream.cpp | 7 +++++++ tests/test_iostream.py | 7 +++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index 44261e881e..1bba6dac8e 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -178,10 +178,18 @@ class scoped_ostream_redirect { old = costream.rdbuf(&buffer); } - ~scoped_ostream_redirect() { costream.rdbuf(old); } + ~scoped_ostream_redirect() { + if (old) { + costream.rdbuf(old); + } + } scoped_ostream_redirect(const scoped_ostream_redirect &) = delete; - scoped_ostream_redirect(scoped_ostream_redirect &&other) = default; + scoped_ostream_redirect(scoped_ostream_redirect &&other) + : old(other.old), costream(other.costream), buffer(std::move(other.buffer)) { + other.old = nullptr; // Disarm moved-from destructor + costream.rdbuf(&buffer); // Re-point stream to our buffer + } scoped_ostream_redirect &operator=(const scoped_ostream_redirect &) = delete; scoped_ostream_redirect &operator=(scoped_ostream_redirect &&) = delete; }; diff --git a/tests/test_iostream.cpp b/tests/test_iostream.cpp index 421eaa2dd8..c64a008196 100644 --- a/tests/test_iostream.cpp +++ b/tests/test_iostream.cpp @@ -123,4 +123,11 @@ TEST_SUBMODULE(iostream, m) { .def("stop", &TestThread::stop) .def("join", &TestThread::join) .def("sleep", &TestThread::sleep); + + m.def("move_redirect_output", [](const std::string &msg) { + py::scoped_ostream_redirect redir1(std::cout, py::module_::import("sys").attr("stdout")); + std::cout << "before" << std::flush; + py::scoped_ostream_redirect redir2(std::move(redir1)); + std::cout << msg << std::flush; + }); } diff --git a/tests/test_iostream.py b/tests/test_iostream.py index 791b9e0483..179ba8e62c 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -284,6 +284,13 @@ def test_redirect_both(capfd): assert stream2.getvalue() == msg2 +def test_move_redirect(capsys): + m.move_redirect_output("after") + stdout, stderr = capsys.readouterr() + assert stdout == "beforeafter" + assert not stderr + + @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_threading(): with m.ostream_redirect(stdout=True, stderr=False): From 9be164e4895251cf06f8df3799ff0aa30db8b200 Mon Sep 17 00:00:00 2001 From: Agisilaos Kounelis Date: Thu, 9 Apr 2026 14:22:27 +0300 Subject: [PATCH 2/6] fix: mark move constructor noexcept to satisfy clang-tidy --- include/pybind11/iostream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index 1bba6dac8e..d6fab79f1f 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -185,7 +185,7 @@ class scoped_ostream_redirect { } scoped_ostream_redirect(const scoped_ostream_redirect &) = delete; - scoped_ostream_redirect(scoped_ostream_redirect &&other) + scoped_ostream_redirect(scoped_ostream_redirect &&other) noexcept : old(other.old), costream(other.costream), buffer(std::move(other.buffer)) { other.old = nullptr; // Disarm moved-from destructor costream.rdbuf(&buffer); // Re-point stream to our buffer From 85969779e86a9d3041bc8c17e5de1cb547e0b8df Mon Sep 17 00:00:00 2001 From: Agisilaos Kounelis Date: Mon, 13 Apr 2026 23:43:56 +0300 Subject: [PATCH 3/6] fix: use bool flag instead of nullptr sentinel for moved-from state Using `old == nullptr` as the moved-from sentinel was incorrect because nullptr is a valid original rdbuf() value (e.g. `std::ostream os(nullptr)`). Replace with an explicit `active` flag so the destructor correctly restores nullptr buffers. Add tests for the nullptr-rdbuf edge case. --- include/pybind11/iostream.h | 5 +++-- tests/test_iostream.cpp | 26 +++++++++++++++++++++++--- tests/test_iostream.py | 15 +++++++++++++-- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index d6fab79f1f..c7a27a22fb 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -169,6 +169,7 @@ class scoped_ostream_redirect { std::streambuf *old; std::ostream &costream; detail::pythonbuf buffer; + bool active = true; public: explicit scoped_ostream_redirect(std::ostream &costream = std::cout, @@ -179,7 +180,7 @@ class scoped_ostream_redirect { } ~scoped_ostream_redirect() { - if (old) { + if (active) { costream.rdbuf(old); } } @@ -187,7 +188,7 @@ class scoped_ostream_redirect { scoped_ostream_redirect(const scoped_ostream_redirect &) = delete; scoped_ostream_redirect(scoped_ostream_redirect &&other) noexcept : old(other.old), costream(other.costream), buffer(std::move(other.buffer)) { - other.old = nullptr; // Disarm moved-from destructor + other.active = false; // Disarm moved-from destructor costream.rdbuf(&buffer); // Re-point stream to our buffer } scoped_ostream_redirect &operator=(const scoped_ostream_redirect &) = delete; diff --git a/tests/test_iostream.cpp b/tests/test_iostream.cpp index c64a008196..d34db45fce 100644 --- a/tests/test_iostream.cpp +++ b/tests/test_iostream.cpp @@ -124,10 +124,30 @@ TEST_SUBMODULE(iostream, m) { .def("join", &TestThread::join) .def("sleep", &TestThread::sleep); - m.def("move_redirect_output", [](const std::string &msg) { + m.def("move_redirect_output", [](const std::string &msg_before, const std::string &msg_after) { py::scoped_ostream_redirect redir1(std::cout, py::module_::import("sys").attr("stdout")); - std::cout << "before" << std::flush; + std::cout << msg_before << std::flush; py::scoped_ostream_redirect redir2(std::move(redir1)); - std::cout << msg << std::flush; + std::cout << msg_after << std::flush; + }); + + // Redirect a stream whose original rdbuf is nullptr, then move the redirect. + // Verifies that nullptr is correctly restored (not confused with a moved-from sentinel). + m.def("move_redirect_null_rdbuf", [](const std::string &msg) { + std::ostream os(nullptr); + py::scoped_ostream_redirect redir1(os, py::module_::import("sys").attr("stdout")); + os << msg << std::flush; + py::scoped_ostream_redirect redir2(std::move(redir1)); + os << msg << std::flush; + // After redir2 goes out of scope, os.rdbuf() should be restored to nullptr. + }); + + m.def("get_null_rdbuf_restored", [](const std::string &msg) -> bool { + std::ostream os(nullptr); + { + py::scoped_ostream_redirect redir(os, py::module_::import("sys").attr("stdout")); + os << msg << std::flush; + } + return os.rdbuf() == nullptr; }); } diff --git a/tests/test_iostream.py b/tests/test_iostream.py index 179ba8e62c..027d27c1f9 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -285,12 +285,23 @@ def test_redirect_both(capfd): def test_move_redirect(capsys): - m.move_redirect_output("after") + m.move_redirect_output("before_move", "after_move") stdout, stderr = capsys.readouterr() - assert stdout == "beforeafter" + assert stdout == "before_moveafter_move" assert not stderr +def test_move_redirect_null_rdbuf(capsys): + m.move_redirect_null_rdbuf("hello") + stdout, stderr = capsys.readouterr() + assert stdout == "hellohello" + assert not stderr + + +def test_null_rdbuf_restored(): + assert m.get_null_rdbuf_restored("test") + + @pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_threading(): with m.ostream_redirect(stdout=True, stderr=False): From 5b0a441a647daff1307ffe087e64055766627461 Mon Sep 17 00:00:00 2001 From: Agisilaos Kounelis Date: Wed, 15 Apr 2026 22:06:18 +0300 Subject: [PATCH 4/6] fix: remove noexcept and propagate active flag from source - Remove noexcept: pythonbuf inherits from std::streambuf whose move is not guaranteed nothrow on all implementations. Suppress clang-tidy with NOLINTNEXTLINE instead. - Initialize active from other.active so that moving an already moved-from object does not incorrectly re-activate the redirect. - Only rebind the stream and disarm the source when active. --- include/pybind11/iostream.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index c7a27a22fb..34e0662b84 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -186,10 +186,14 @@ class scoped_ostream_redirect { } scoped_ostream_redirect(const scoped_ostream_redirect &) = delete; - scoped_ostream_redirect(scoped_ostream_redirect &&other) noexcept - : old(other.old), costream(other.costream), buffer(std::move(other.buffer)) { - other.active = false; // Disarm moved-from destructor - costream.rdbuf(&buffer); // Re-point stream to our buffer + // NOLINTNEXTLINE(performance-noexcept-move-constructor) + scoped_ostream_redirect(scoped_ostream_redirect &&other) + : old(other.old), costream(other.costream), buffer(std::move(other.buffer)), + active(other.active) { + if (active) { + costream.rdbuf(&buffer); // Re-point stream to our buffer + other.active = false; + } } scoped_ostream_redirect &operator=(const scoped_ostream_redirect &) = delete; scoped_ostream_redirect &operator=(scoped_ostream_redirect &&) = delete; From 58a45b57c6f4ba3a9d93578d9db4f8a00588088b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 15 Apr 2026 15:05:56 -0700 Subject: [PATCH 5/6] test: add unflushed ostream redirect regression Cover the buffered-before-move case for `scoped_ostream_redirect`, which still crashes despite the current move fix. This gives the PR a direct reproducer for the remaining bug path. Made-with: Cursor --- tests/test_iostream.cpp | 9 +++++++++ tests/test_iostream.py | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/tests/test_iostream.cpp b/tests/test_iostream.cpp index d34db45fce..7484e734be 100644 --- a/tests/test_iostream.cpp +++ b/tests/test_iostream.cpp @@ -131,6 +131,15 @@ TEST_SUBMODULE(iostream, m) { std::cout << msg_after << std::flush; }); + m.def("move_redirect_output_unflushed", + [](const std::string &msg_before, const std::string &msg_after) { + py::scoped_ostream_redirect redir1(std::cout, + py::module_::import("sys").attr("stdout")); + std::cout << msg_before; + py::scoped_ostream_redirect redir2(std::move(redir1)); + std::cout << msg_after << std::flush; + }); + // Redirect a stream whose original rdbuf is nullptr, then move the redirect. // Verifies that nullptr is correctly restored (not confused with a moved-from sentinel). m.def("move_redirect_null_rdbuf", [](const std::string &msg) { diff --git a/tests/test_iostream.py b/tests/test_iostream.py index 027d27c1f9..857e0b5f73 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -291,6 +291,13 @@ def test_move_redirect(capsys): assert not stderr +def test_move_redirect_unflushed(capsys): + m.move_redirect_output_unflushed("before_move", "after_move") + stdout, stderr = capsys.readouterr() + assert stdout == "before_moveafter_move" + assert not stderr + + def test_move_redirect_null_rdbuf(capsys): m.move_redirect_null_rdbuf("hello") stdout, stderr = capsys.readouterr() From e84bd709797856eeddee10cb4a3883fc51e89ba1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 15 Apr 2026 15:18:12 -0700 Subject: [PATCH 6/6] fix: disarm moved-from pythonbuf after redirect move The redirect guard now survives moves, but buffered output could still remain in the moved-from `pythonbuf` and be flushed during destruction through moved-out Python handles. Rebuild the destination put area from the transferred storage and clear the source put area so unflushed bytes follow the active redirect instead of crashing in the moved-from destructor. Made-with: Cursor --- include/pybind11/iostream.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index 34e0662b84..df7fa3c381 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -131,7 +131,22 @@ class pythonbuf : public std::streambuf { setp(d_buffer.get(), d_buffer.get() + buf_size - 1); } - pythonbuf(pythonbuf &&) = default; + pythonbuf(pythonbuf &&other) noexcept + : buf_size(other.buf_size), d_buffer(std::move(other.d_buffer)), + pywrite(std::move(other.pywrite)), pyflush(std::move(other.pyflush)) { + const auto pending = (other.pbase() != nullptr && other.pptr() != nullptr) + ? static_cast(other.pptr() - other.pbase()) + : 0; + if (d_buffer != nullptr) { + // Rebuild the put area from the transferred storage. + setp(d_buffer.get(), d_buffer.get() + buf_size - 1); + pbump(pending); + } else { + setp(nullptr, nullptr); + } + // Prevent the moved-from destructor from flushing through moved-out handles. + other.setp(nullptr, nullptr); + } /// Sync before destroy ~pythonbuf() override { _sync(); }