Skip to content

Commit 74079d9

Browse files
authored
runtime: flush stdout and stderr file descriptors at exit (#38)
2 parents 4d9abd5 + e97e80c commit 74079d9

10 files changed

Lines changed: 490 additions & 157 deletions

File tree

integration/run_python_tests.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,19 @@ for file in $(find $SCRIPT_DIR/tests/ -maxdepth 1 -type f -name "*.py"); do
3131
fi
3232
done
3333

34+
# An uncaught exception must exit non-zero, and the exit-time flush of the
35+
# buffered sys.stdout must run before the traceback is printed, so with a
36+
# redirected stdout the script's own output comes first.
37+
file=$SCRIPT_DIR/tests/expected_failures/print_then_raise.py
38+
output=$(timeout 10s $PYTHON_EXECUTABLE $file --gc-frequency $GC_FREQUENCY 2>&1)
39+
if [ $? -eq 0 ]; then
40+
echo $file "... FAILED! (expected a non-zero exit code)"
41+
exit_code=1
42+
elif [ "$(echo "$output" | head -n 1)" != "before-raise" ]; then
43+
echo $file "... FAILED! (script output must precede the traceback, got: ${output})"
44+
exit_code=1
45+
else
46+
echo $file "... PASSED!"
47+
fi
48+
3449
exit $exit_code
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# BufferedWriter must respect buffer_size like CPython (Modules/_io/bufferedio.c):
2+
# - writes that fit in the buffer's free space are only buffered (no raw write)
3+
# - writes that don't fit first drain the buffer to the raw stream
4+
# - payloads larger than buffer_size bypass the buffer and go straight to raw
5+
# - the remaining tail (<= buffer_size) is buffered again
6+
import _io
7+
8+
PATH = "/tmp/pycpp_buffered_writer_test.bin"
9+
10+
11+
def file_contents():
12+
f = _io.FileIO(PATH, "rb")
13+
data = f.readall()
14+
f.close()
15+
return data
16+
17+
18+
# 1. a small write stays in the buffer until flush
19+
w = _io.BufferedWriter(_io.FileIO(PATH, "wb"))
20+
assert w.write(b"abcd") == 4
21+
assert file_contents() == b"", file_contents()
22+
w.flush()
23+
assert file_contents() == b"abcd", file_contents()
24+
25+
# 2. overflowing the buffer drains it, and an oversized payload bypasses the buffer
26+
w = _io.BufferedWriter(_io.FileIO(PATH, "wb"), 8)
27+
assert w.write(b"abcd") == 4
28+
assert file_contents() == b"", file_contents()
29+
assert w.write(b"efghijklm") == 9
30+
assert file_contents() == b"abcdefghijklm", file_contents()
31+
32+
# 3. a tail smaller than buffer_size stays buffered after the drain
33+
w = _io.BufferedWriter(_io.FileIO(PATH, "wb"), 8)
34+
assert w.write(b"abcdef") == 6
35+
assert w.write(b"ghi") == 3
36+
assert file_contents() == b"abcdef", file_contents()
37+
w.flush()
38+
assert file_contents() == b"abcdefghi", file_contents()
39+
40+
# 4. an exact fit is fully buffered
41+
w = _io.BufferedWriter(_io.FileIO(PATH, "wb"), 8)
42+
assert w.write(b"12345678") == 8
43+
assert file_contents() == b"", file_contents()
44+
w.flush()
45+
assert file_contents() == b"12345678", file_contents()
46+
47+
# 5. buffer_size must be strictly positive
48+
try:
49+
_io.BufferedWriter(_io.FileIO(PATH, "wb"), 0)
50+
assert False, "expected ValueError"
51+
except ValueError:
52+
pass
53+
54+
55+
# 6. raw can be any duck-typed object; a write() that over-reports the byte
56+
# count must raise OSError instead of wrapping the unsigned byte counters
57+
class OverReportingWriter:
58+
def write(self, b):
59+
return 1000
60+
61+
62+
w = _io.BufferedWriter(OverReportingWriter(), 8)
63+
try:
64+
w.write(b"0123456789abcdef")
65+
assert False, "expected OSError"
66+
except OSError:
67+
pass
68+
69+
70+
# 7. same for a negative count
71+
class NegativeWriter:
72+
def write(self, b):
73+
return -1
74+
75+
76+
w = _io.BufferedWriter(NegativeWriter(), 8)
77+
try:
78+
w.write(b"0123456789abcdef")
79+
assert False, "expected OSError"
80+
except OSError:
81+
pass
82+
83+
# 8. a raw write() returning 0 makes no progress; retrying forever would hang,
84+
# so it must raise OSError
85+
class ZeroWriter:
86+
def write(self, b):
87+
return 0
88+
89+
90+
w = _io.BufferedWriter(ZeroWriter(), 8)
91+
try:
92+
w.write(b"0123456789abcdef")
93+
assert False, "expected OSError"
94+
except OSError:
95+
pass
96+
97+
98+
# 9. a raw write() returning None means the stream accepted no data without
99+
# blocking; that is an OSError (BlockingIOError in CPython), not a crash
100+
class NoneWriter:
101+
def write(self, b):
102+
return None
103+
104+
105+
w = _io.BufferedWriter(NoneWriter(), 8)
106+
try:
107+
w.write(b"0123456789abcdef")
108+
assert False, "expected OSError"
109+
except OSError:
110+
pass
111+
112+
113+
# 10. any other non-int return from raw write() is a TypeError, not a crash
114+
class StringWriter:
115+
def write(self, b):
116+
return "16"
117+
118+
119+
w = _io.BufferedWriter(StringWriter(), 8)
120+
try:
121+
w.write(b"0123456789abcdef")
122+
assert False, "expected TypeError"
123+
except TypeError:
124+
pass
125+
126+
# 11. an object created via __new__ without __init__ has no raw stream; every
127+
# I/O method must raise ValueError instead of dereferencing it
128+
uninitialized = _io.BufferedWriter.__new__(_io.BufferedWriter)
129+
for method in (
130+
uninitialized.isatty,
131+
uninitialized.flush,
132+
lambda: uninitialized.write(b"x"),
133+
):
134+
try:
135+
method()
136+
assert False, "expected ValueError"
137+
except ValueError:
138+
pass
139+
140+
# 12. the oversized path calls back into Python once per chunk; a raw whose
141+
# write() reallocates the source object's storage must not leave the write
142+
# loop walking freed memory
143+
ba = bytearray(b"A" * 100)
144+
chunks = []
145+
146+
147+
class MutatingWriter:
148+
def write(self, b):
149+
# reallocates ba's backing storage mid-write
150+
ba[0:1] = b"B" * 200000
151+
chunks.append(1)
152+
return 1
153+
154+
155+
w = _io.BufferedWriter(MutatingWriter(), 8)
156+
assert w.write(ba) == 100
157+
# 100 bytes, one accepted per call, until the 8 byte tail is buffered instead
158+
assert len(chunks) == 92, len(chunks)
159+
160+
print("buffered_writer: ok")
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Buffered sys.stdout must be flushed by the interpreter's exit callbacks
2+
# *before* the uncaught-exception traceback is printed, so with a redirected
3+
# stdout the script's own output comes first. run_python_tests.sh checks the
4+
# output ordering and that the exit code is non-zero.
5+
print("before-raise")
6+
raise RuntimeError("boom")

src/executable/bytecode/BytecodeProgram.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#include "Bytecode.hpp"
33
#include "executable/Function.hpp"
44
#include "executable/Mangler.hpp"
5-
#include "interpreter/InterpreterSession.hpp"
5+
#include "interpreter/Interpreter.hpp"
66
#include "runtime/PyCode.hpp"
77
#include "runtime/PyFrame.hpp"
88
#include "runtime/PyFunction.hpp"
@@ -136,6 +136,11 @@ int BytecodeProgram::execute(VirtualMachine *vm)
136136

137137
auto result = m_main_function->function()->call(*vm, interpreter);
138138

139+
{
140+
ScopedStack scoped_stack{ vm->push_frame(1, 0, 0) };
141+
[[maybe_unused]] auto final_result = interpreter.finalise();
142+
}
143+
139144
if (result.is_err()) {
140145
// The exception propagated all the way out; the eval loop already popped it
141146
// off the (now-clean) exception stack as it unwound, so use the result value

src/interpreter/Interpreter.cpp

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "runtime/Import.hpp"
55
#include "runtime/KeyError.hpp"
66
#include "runtime/NameError.hpp"
7+
#include "runtime/PyBool.hpp"
78
#include "runtime/PyCode.hpp"
89
#include "runtime/PyDict.hpp"
910
#include "runtime/PyFrame.hpp"
@@ -26,6 +27,7 @@
2627

2728
#include <cstdio>
2829
#include <filesystem>
30+
#include <ranges>
2931
#include <unistd.h>
3032

3133
namespace fs = std::filesystem;
@@ -209,8 +211,13 @@ void Interpreter::internal_setup(const std::string &name,
209211
return buffered_writer->call(PyTuple::create(stdout).unwrap(), nullptr);
210212
})
211213
.and_then([text_io_wrapper](PyObject *stdout_buffer_writer) {
214+
PyObject *line_buffering =
215+
::isatty(STDOUT_FILENO) == 1 ? py_true() : py_false();
212216
return text_io_wrapper->call(
213-
PyTuple::create(stdout_buffer_writer).unwrap(), nullptr);
217+
PyTuple::create(
218+
stdout_buffer_writer, py_none(), py_none(), py_none(), line_buffering)
219+
.unwrap(),
220+
nullptr);
214221
});
215222
ASSERT(py_stdout.is_ok());
216223
auto py_stderr =
@@ -220,13 +227,30 @@ void Interpreter::internal_setup(const std::string &name,
220227
return buffered_writer->call(PyTuple::create(stderr).unwrap(), nullptr);
221228
})
222229
.and_then([text_io_wrapper](PyObject *stderr_buffer_writer) {
230+
auto *line_buffering = py_true();
223231
return text_io_wrapper->call(
224-
PyTuple::create(stderr_buffer_writer).unwrap(), nullptr);
232+
PyTuple::create(
233+
stderr_buffer_writer, py_none(), py_none(), py_none(), line_buffering)
234+
.unwrap(),
235+
nullptr);
225236
});
226237
ASSERT(py_stderr.is_ok());
227-
sys->add_symbol(PyString::create("stdin").unwrap(), py_stdin.unwrap());
228-
sys->add_symbol(PyString::create("stdout").unwrap(), py_stdout.unwrap());
229-
sys->add_symbol(PyString::create("stderr").unwrap(), py_stderr.unwrap());
238+
239+
for (auto [name, obj] : std::views::zip(std::array{ "stdin", "stdout", "stderr" },
240+
std::array{ py_stdin.unwrap(), py_stdout.unwrap(), py_stderr.unwrap() })) {
241+
sys->add_symbol(PyString::create(name).unwrap(), obj);
242+
register_callback(PyNativeFunction::create(
243+
std::string{ name } + "_exit",
244+
[obj](PyTuple *, PyDict *) -> PyResult<PyObject *> {
245+
return obj->get_method(PyString::create("flush").unwrap())
246+
.and_then([](PyObject *flush) {
247+
return flush->call(nullptr, nullptr);
248+
});
249+
},
250+
obj)
251+
.unwrap(),
252+
nullptr);
253+
}
230254
}
231255

232256
if (config.requires_importlib) {
@@ -489,4 +513,32 @@ void Interpreter::visit_graph(::Cell::Visitor &visitor)
489513
if (m_codec_error_registry) visitor.visit(*m_codec_error_registry);
490514
if (m_codec_search_path) visitor.visit(*m_codec_search_path);
491515
if (m_codec_search_path_cache) visitor.visit(*m_codec_search_path_cache);
516+
517+
for (auto [callback, args] : m_callbacks) {
518+
if (callback) visitor.visit(*callback);
519+
if (args) visitor.visit(*args);
520+
}
521+
}
522+
523+
void Interpreter::register_callback(PyObject *callback, PyTuple *args)
524+
{
525+
m_callbacks.emplace_back(callback, args);
526+
}
527+
528+
void Interpreter::unregister_callback(PyObject *callback)
529+
{
530+
m_callbacks.erase(
531+
std::ranges::remove(m_callbacks, callback, [](const auto &el) { return std::get<0>(el); })
532+
.begin(),
533+
m_callbacks.end());
534+
}
535+
536+
PyResult<std::monostate> Interpreter::finalise()
537+
{
538+
// TODO: capture exceptions, and raise last one
539+
for (auto [callback, args] : m_callbacks) {
540+
[[maybe_unused]] auto result = callback->call(args, nullptr);
541+
}
542+
543+
return Ok(std::monostate{});
492544
}

src/interpreter/Interpreter.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <string>
88
#include <string_view>
9+
#include <variant>
910

1011
class BytecodeProgram;
1112

@@ -34,6 +35,7 @@ class Interpreter
3435
py::PyDict *m_codec_search_path_cache{ nullptr };
3536
std::string m_entry_script;
3637
std::vector<std::string> m_argv;
38+
std::vector<std::tuple<py::PyObject *, py::PyTuple *>> m_callbacks;
3739

3840
public:
3941
struct Config
@@ -89,6 +91,8 @@ class Interpreter
8991
void setup(std::shared_ptr<BytecodeProgram> &&program);
9092
void setup_main_interpreter(std::shared_ptr<BytecodeProgram> &&program);
9193

94+
py::PyResult<std::monostate> finalise();
95+
9296
const std::string &entry_script() const { return m_entry_script; }
9397
const std::vector<std::string> &argv() const { return m_argv; }
9498

@@ -107,6 +111,10 @@ class Interpreter
107111
py::PyTuple *args,
108112
py::PyDict *kwargs);
109113

114+
115+
void register_callback(py::PyObject *callback, py::PyTuple *args);
116+
void unregister_callback(py::PyObject *callback);
117+
110118
void visit_graph(::Cell::Visitor &);
111119

112120
private:

src/repl/repl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,4 @@ int main(int argc, char **argv)
230230
// }
231231

232232
return EXIT_SUCCESS;
233-
}
233+
}

src/runtime/modules/BuiltinsModule.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ PyResult<PyObject *> print(const PyTuple *args, const PyDict *kwargs, Interprete
9292
// sys.stdout may be None when FILE* stdout isn't connected
9393
if (!file || file == py_none()) { return Ok(py_none()); }
9494
}
95-
// TODO: flush should be false, but for now we keep it as true, since there is no flush at
96-
// interpreter shutdown
97-
bool flush = true;
95+
bool flush = false;
9896
if (kwargs) {
9997
static const Value separator_keyword = String{ "sep" };
10098
static const Value end_keyword = String{ "end" };

src/runtime/modules/IOChecks.hpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#pragma once
2+
3+
#include "runtime/Value.hpp"
4+
#include "runtime/ValueError.hpp"
5+
6+
#include <variant>
7+
8+
namespace py {
9+
10+
// bool is_initialized() const - required by check_initialized()
11+
// bool is_detached() const - optional; reported separately when the object was
12+
// initialized once and then detached
13+
template<typename Derived> class IOChecks
14+
{
15+
const Derived &derived() const { return static_cast<const Derived &>(*this); }
16+
17+
public:
18+
PyResult<std::monostate> check_initialized() const
19+
{
20+
if (derived().is_initialized()) { return Ok(std::monostate{}); }
21+
if constexpr (requires(const Derived &d) { d.is_detached(); }) {
22+
if (derived().is_detached()) {
23+
return Err(value_error("raw stream has been detached"));
24+
}
25+
}
26+
return Err(value_error("I/O operation on uninitialized object"));
27+
}
28+
29+
// the guard pair an accessor wants: usable means constructed *and* still open.
30+
// Spelling it once keeps the two checks from being chained in the wrong order.
31+
PyResult<std::monostate> check_usable() const
32+
{
33+
return check_initialized().and_then([this](auto) { return derived().check_closed(); });
34+
}
35+
};
36+
37+
}// namespace py

0 commit comments

Comments
 (0)