Skip to content

Commit 4b1023b

Browse files
committed
Synchronize access to zstream to prevent segfault in multithreaded use
I'm not sure whether this handles all multithreaded use cases, but this handles the example that crashes almost immediately and does 10,000,000 total deflates using 100 separate threads. To prevent the tests from taking forever, the committed test for this uses only 10,000 deflates across 10 separate threads, which still causes a segfault in the previous implementation almost immediately. Fixes [Bug #17803]
1 parent b9fddcc commit 4b1023b

File tree

2 files changed

+93
-1
lines changed

2 files changed

+93
-1
lines changed

ext/zlib/zlib.c

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ struct zstream {
546546
unsigned long flags;
547547
VALUE buf;
548548
VALUE input;
549+
VALUE mutex;
549550
z_stream stream;
550551
const struct zstream_funcs {
551552
int (*reset)(z_streamp);
@@ -621,6 +622,7 @@ zstream_init(struct zstream *z, const struct zstream_funcs *func)
621622
z->flags = 0;
622623
z->buf = Qnil;
623624
z->input = Qnil;
625+
z->mutex = rb_mutex_new();
624626
z->stream.zalloc = zlib_mem_alloc;
625627
z->stream.zfree = zlib_mem_free;
626628
z->stream.opaque = Z_NULL;
@@ -652,7 +654,9 @@ zstream_expand_buffer(struct zstream *z)
652654
rb_obj_reveal(z->buf, rb_cString);
653655
}
654656

657+
rb_mutex_unlock(z->mutex);
655658
rb_protect(rb_yield, z->buf, &state);
659+
rb_mutex_lock(z->mutex);
656660

657661
if (ZSTREAM_REUSE_BUFFER_P(z)) {
658662
rb_str_modify(z->buf);
@@ -1054,7 +1058,7 @@ zstream_unblock_func(void *ptr)
10541058
}
10551059

10561060
static void
1057-
zstream_run(struct zstream *z, Bytef *src, long len, int flush)
1061+
zstream_run0(struct zstream *z, Bytef *src, long len, int flush)
10581062
{
10591063
struct zstream_run_args args;
10601064
int err;
@@ -1138,6 +1142,32 @@ zstream_run(struct zstream *z, Bytef *src, long len, int flush)
11381142
rb_jump_tag(args.jump_state);
11391143
}
11401144

1145+
struct zstream_run_synchronized_args {
1146+
struct zstream *z;
1147+
Bytef *src;
1148+
long len;
1149+
int flush;
1150+
};
1151+
1152+
static VALUE
1153+
zstream_run_synchronized(VALUE value_arg)
1154+
{
1155+
struct zstream_run_synchronized_args *run_args = (struct zstream_run_synchronized_args *)value_arg;
1156+
zstream_run0(run_args->z, run_args->src, run_args->len, run_args->flush);
1157+
return Qnil;
1158+
}
1159+
1160+
static void
1161+
zstream_run(struct zstream *z, Bytef *src, long len, int flush)
1162+
{
1163+
struct zstream_run_synchronized_args run_args;
1164+
run_args.z = z;
1165+
run_args.src = src;
1166+
run_args.len = len;
1167+
run_args.flush = flush;
1168+
rb_mutex_synchronize(z->mutex, zstream_run_synchronized, (VALUE)&run_args);
1169+
}
1170+
11411171
static VALUE
11421172
zstream_sync(struct zstream *z, Bytef *src, long len)
11431173
{
@@ -1183,6 +1213,7 @@ zstream_mark(void *p)
11831213
struct zstream *z = p;
11841214
rb_gc_mark(z->buf);
11851215
rb_gc_mark(z->input);
1216+
rb_gc_mark(z->mutex);
11861217
}
11871218

11881219
static void

test/zlib/test_zlib.rb

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require 'stringio'
55
require 'tempfile'
66
require 'tmpdir'
7+
require 'securerandom'
78

89
begin
910
require 'zlib'
@@ -503,6 +504,66 @@ def test_set_dictionary
503504
assert_raise(Zlib::StreamError) { z.set_dictionary("foo") }
504505
z.close
505506
end
507+
508+
def test_multithread_deflate
509+
zd = Zlib::Deflate.new
510+
511+
s = "x" * 10000
512+
(0...10).map do |x|
513+
Thread.new do
514+
1000.times { zd.deflate(s) }
515+
end
516+
end.each do |th|
517+
th.join
518+
end
519+
ensure
520+
zd&.finish
521+
zd&.close
522+
end
523+
524+
def test_multithread_inflate
525+
zi = Zlib::Inflate.new
526+
527+
s = Zlib.deflate("x" * 10000)
528+
(0...10).map do |x|
529+
Thread.new do
530+
1000.times { zi.inflate(s) }
531+
end
532+
end.each do |th|
533+
th.join
534+
end
535+
ensure
536+
zi&.finish
537+
zi&.close
538+
end
539+
540+
def test_recursive_deflate
541+
zd = Zlib::Deflate.new
542+
543+
s = SecureRandom.random_bytes(1024**2)
544+
assert_raise(Zlib::BufError) do
545+
zd.deflate(s) do
546+
zd.deflate(s)
547+
end
548+
end
549+
ensure
550+
zd&.finish
551+
zd&.close
552+
end
553+
554+
def test_recursive_inflate
555+
zi = Zlib::Inflate.new
556+
557+
s = Zlib.deflate(SecureRandom.random_bytes(1024**2))
558+
559+
assert_raise(Zlib::DataError) do
560+
zi.inflate(s) do
561+
zi.inflate(s)
562+
end
563+
end
564+
ensure
565+
zi&.close
566+
end
506567
end
507568

508569
class TestZlibGzipFile < Test::Unit::TestCase

0 commit comments

Comments
 (0)