pythonbuf fix

include/pybind11/iostream.h

@@ -43,11 +43,15 @@ class pythonbuf : public std::streambuf {
     // simplified to a fully qualified call.
     int _sync() {
         if (pbase() != pptr()) {
-            // This subtraction cannot be negative, so dropping the sign
-            str line(pbase(), static_cast<size_t>(pptr() - pbase()));
 
             {
                 gil_scoped_acquire tmp;
+
+                // This subtraction cannot be negative, so dropping the sign.
+                // This invokes python and therefore should be included in the
+                // GIL
+                str line(pbase(), static_cast<size_t>(pptr() - pbase()));
+
                 pywrite(line);
                 pyflush();
             }

tests/test_iostream.cpp

@@ -11,6 +11,8 @@
 #include <pybind11/iostream.h>
 #include "pybind11_tests.h"
 #include <iostream>
+#include <thread>
+#include <atomic>
 
 
 void noisy_function(std::string msg, bool flush) {
@@ -25,6 +27,40 @@ void noisy_funct_dual(std::string msg, std::string emsg) {
     std::cerr << emsg;
 }
 
+// object to manage C++ thread
+// simply repeatedly write to std::cerr until stopped
+// redirect is called at some point to test the safety of scoped_estream_redirect
+struct TestThread {
+    TestThread() : t_{nullptr}, stop_{false} {
+        auto thread_f = [this] {
+            while(!this->stop_) {
+                std::cout << "x" << std::flush;
+                std::this_thread::sleep_for(std::chrono::microseconds(50));
+            } };
+        t_ = new std::thread(std::move(thread_f));
+    }
+
+    ~TestThread() {
+        delete t_;
+    }
+
+    void stop() { stop_ = 1; }
+
+    void join() {
+        py::gil_scoped_release gil_lock;
+        t_->join();
+    }
+
+    void sleep() {
+        py::gil_scoped_release gil_lock;
+        std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    }
+
+    std::thread * t_;
+    std::atomic<bool> stop_;
+};
+
+
 TEST_SUBMODULE(iostream, m) {
 
     add_ostream_redirect(m);
@@ -70,4 +106,13 @@ TEST_SUBMODULE(iostream, m) {
         std::cout << msg << std::flush;
         std::cerr << emsg << std::flush;
     });
+
+    py::class_<TestThread>(m, "TestThread")
+        .def(py::init<>())
+
+        .def("stop", &TestThread::stop)
+
+        .def("join", &TestThread::join)
+
+        .def("sleep", &TestThread::sleep);
 }

tests/test_iostream.py

@@ -216,3 +216,26 @@ def test_redirect_both(capfd):
     assert stderr == ""
     assert stream.getvalue() == msg
     assert stream2.getvalue() == msg2
+
+
+def test_threading():
+    with m.ostream_redirect(stdout=True, stderr=False):
+        # start some threads
+        threads = []
+
+        # start some threads
+        for _j in range(20):
+            threads.append(m.TestThread())
+
+        # give the threads some time to fail
+        threads[0].sleep()
+
+        # stop all the threads
+        for t in threads:
+            t.stop()
+
+        for t in threads:
+            t.join()
+
+        # if a thread segfaults, we don't get here
+        assert True