Skip to content

Commit

Permalink
Fix streaming to file descriptor
Browse files Browse the repository at this point in the history
Don't use write(2) for writing to file descriptors, use IO::write
instead just as for writing to a non-file IO.

Using write(2) means having to handle all kinds of edge cases, such as
EINTR and EAGAIN/EWOULDBLOCK, and doesn't release the GVL when
blocking.
  • Loading branch information
jscheid committed Dec 11, 2024
1 parent c70bf41 commit 903284a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
8 changes: 2 additions & 6 deletions ext/oj/stream_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ static void stream_writer_write(StreamWriter sw) {

switch (sw->type) {
case STRING_IO:
case STREAM_IO: {
case STREAM_IO:
case FILE_IO: {
volatile VALUE rs = rb_str_new(sw->sw.out.buf, size);

// Oddly enough, when pushing ASCII characters with UTF-8 encoding or
Expand All @@ -53,11 +54,6 @@ static void stream_writer_write(StreamWriter sw) {
rb_funcall(sw->stream, oj_write_id, 1, rs);
break;
}
case FILE_IO:
if (size != write(sw->fd, sw->sw.out.buf, size)) {
rb_raise(rb_eIOError, "Write failed. [_%d_:%s]\n", errno, strerror(errno));
}
break;
default: rb_raise(rb_eArgError, "expected an IO Object.");
}
stream_writer_reset_buf(sw);
Expand Down
14 changes: 14 additions & 0 deletions test/test_writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
$LOAD_PATH << __dir__

require 'helper'
require 'open3'

class OjWriter < Minitest::Test

Expand Down Expand Up @@ -377,4 +378,17 @@ def test_stream_writer_push_null_value_with_key
w.pop()
assert_equal(%|{"nothing":null}\n|, output.string())
end

def test_stream_writer_subprocess
Open3.popen3("/bin/bash", "-c", "cat > /dev/null") do |stdin, _stdout, _stderr, _wait_thr|
w = Oj::StreamWriter.new(stdin, :indent => 0)
w.push_array()
chunk = "{\"foo\":\"#{"bar"*1000}\"}"
1000.times do |_|
w.push_json(chunk)
end
w.pop()
stdin.close
end
end
end # OjWriter

0 comments on commit 903284a

Please sign in to comment.