Skip to content

Commit 8596977

Browse files
committed
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.
1 parent 9be164e commit 8596977

File tree

3 files changed

+39
-7
lines changed

3 files changed

+39
-7
lines changed

include/pybind11/iostream.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ class scoped_ostream_redirect {
169169
std::streambuf *old;
170170
std::ostream &costream;
171171
detail::pythonbuf buffer;
172+
bool active = true;
172173

173174
public:
174175
explicit scoped_ostream_redirect(std::ostream &costream = std::cout,
@@ -179,15 +180,15 @@ class scoped_ostream_redirect {
179180
}
180181

181182
~scoped_ostream_redirect() {
182-
if (old) {
183+
if (active) {
183184
costream.rdbuf(old);
184185
}
185186
}
186187

187188
scoped_ostream_redirect(const scoped_ostream_redirect &) = delete;
188189
scoped_ostream_redirect(scoped_ostream_redirect &&other) noexcept
189190
: old(other.old), costream(other.costream), buffer(std::move(other.buffer)) {
190-
other.old = nullptr; // Disarm moved-from destructor
191+
other.active = false; // Disarm moved-from destructor
191192
costream.rdbuf(&buffer); // Re-point stream to our buffer
192193
}
193194
scoped_ostream_redirect &operator=(const scoped_ostream_redirect &) = delete;

tests/test_iostream.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,30 @@ TEST_SUBMODULE(iostream, m) {
124124
.def("join", &TestThread::join)
125125
.def("sleep", &TestThread::sleep);
126126

127-
m.def("move_redirect_output", [](const std::string &msg) {
127+
m.def("move_redirect_output", [](const std::string &msg_before, const std::string &msg_after) {
128128
py::scoped_ostream_redirect redir1(std::cout, py::module_::import("sys").attr("stdout"));
129-
std::cout << "before" << std::flush;
129+
std::cout << msg_before << std::flush;
130130
py::scoped_ostream_redirect redir2(std::move(redir1));
131-
std::cout << msg << std::flush;
131+
std::cout << msg_after << std::flush;
132+
});
133+
134+
// Redirect a stream whose original rdbuf is nullptr, then move the redirect.
135+
// Verifies that nullptr is correctly restored (not confused with a moved-from sentinel).
136+
m.def("move_redirect_null_rdbuf", [](const std::string &msg) {
137+
std::ostream os(nullptr);
138+
py::scoped_ostream_redirect redir1(os, py::module_::import("sys").attr("stdout"));
139+
os << msg << std::flush;
140+
py::scoped_ostream_redirect redir2(std::move(redir1));
141+
os << msg << std::flush;
142+
// After redir2 goes out of scope, os.rdbuf() should be restored to nullptr.
143+
});
144+
145+
m.def("get_null_rdbuf_restored", [](const std::string &msg) -> bool {
146+
std::ostream os(nullptr);
147+
{
148+
py::scoped_ostream_redirect redir(os, py::module_::import("sys").attr("stdout"));
149+
os << msg << std::flush;
150+
}
151+
return os.rdbuf() == nullptr;
132152
});
133153
}

tests/test_iostream.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,23 @@ def test_redirect_both(capfd):
285285

286286

287287
def test_move_redirect(capsys):
288-
m.move_redirect_output("after")
288+
m.move_redirect_output("before_move", "after_move")
289289
stdout, stderr = capsys.readouterr()
290-
assert stdout == "beforeafter"
290+
assert stdout == "before_moveafter_move"
291291
assert not stderr
292292

293293

294+
def test_move_redirect_null_rdbuf(capsys):
295+
m.move_redirect_null_rdbuf("hello")
296+
stdout, stderr = capsys.readouterr()
297+
assert stdout == "hellohello"
298+
assert not stderr
299+
300+
301+
def test_null_rdbuf_restored():
302+
assert m.get_null_rdbuf_restored("test")
303+
304+
294305
@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads")
295306
def test_threading():
296307
with m.ostream_redirect(stdout=True, stderr=False):

0 commit comments

Comments
 (0)