fix: segfault when moving scoped_ostream_redirect (#6033)

* 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.

* fix: mark move constructor noexcept to satisfy clang-tidy

* 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.

* 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.

* 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

* 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

---------

Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>

Author: kounelisagis

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<int>(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(); }
@@ -169,6 +184,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,
@@ -178,10 +194,22 @@ class scoped_ostream_redirect {
         old = costream.rdbuf(&buffer);
     }
 
-    ~scoped_ostream_redirect() { costream.rdbuf(old); }
+    ~scoped_ostream_redirect() {
+        if (active) {
+            costream.rdbuf(old);
+        }
+    }
 
     scoped_ostream_redirect(const scoped_ostream_redirect &) = delete;
-    scoped_ostream_redirect(scoped_ostream_redirect &&other) = default;
+    // 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;
 };

tests/test_iostream.cpp

@@ -123,4 +123,40 @@ 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_before, const std::string &msg_after) {
+        py::scoped_ostream_redirect redir1(std::cout, py::module_::import("sys").attr("stdout"));
+        std::cout << msg_before << std::flush;
+        py::scoped_ostream_redirect redir2(std::move(redir1));
+        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) {
+        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;
+    });
 }

tests/test_iostream.py

@@ -284,6 +284,31 @@ def test_redirect_both(capfd):
     assert stream2.getvalue() == msg2
 
 
+def test_move_redirect(capsys):
+    m.move_redirect_output("before_move", "after_move")
+    stdout, stderr = capsys.readouterr()
+    assert stdout == "before_moveafter_move"
+    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()
+    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):