diff --git a/CHANGELOG.md b/CHANGELOG.md index a2e5e8b..d1c86b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- Fix a potential crash in `hiredis-client` when using subcriptions (`next_event`). See #221. + # 0.23.0 - Allow `password` to be a callable. Makes it easy to implement short lived password authentication strategies. diff --git a/hiredis-client/ext/redis_client/hiredis/hiredis_connection.c b/hiredis-client/ext/redis_client/hiredis/hiredis_connection.c index dc91296..5cbcccc 100644 --- a/hiredis-client/ext/redis_client/hiredis/hiredis_connection.c +++ b/hiredis-client/ext/redis_client/hiredis/hiredis_connection.c @@ -321,6 +321,7 @@ typedef struct { struct timeval connect_timeout; struct timeval read_timeout; struct timeval write_timeout; + hiredis_reader_state_t reader_state; } hiredis_connection_t; static void hiredis_connection_free(void *ptr) { @@ -720,16 +721,14 @@ static VALUE hiredis_flush(VALUE self) { static int hiredis_read_internal(hiredis_connection_t *connection, VALUE *reply) { void *redis_reply = NULL; - // This struct being on the stack, the GC won't move nor collect that `stack` RArray. + // This array being on the stack, the GC won't move nor collect it. // We use that to avoid having to have a `mark` function with write barriers. // Not that it would be too hard, but if we mark the response objects, we'll likely end up // promoting them to the old generation which isn't desirable. VALUE stack = rb_ary_new(); - hiredis_reader_state_t reader_state = { - .stack = stack, - .task_index = &connection->context->reader->ridx, - }; - connection->context->reader->privdata = &reader_state; + connection->reader_state.stack = stack; + connection->reader_state.task_index = &connection->context->reader->ridx; + connection->context->reader->privdata = &connection->reader_state; /* Try to read pending replies */ if (redisGetReplyFromReader(connection->context, &redis_reply) == REDIS_ERR) { diff --git a/test/redis_client/subscriptions_test.rb b/test/redis_client/subscriptions_test.rb index 2c6c41c..48fedc4 100644 --- a/test/redis_client/subscriptions_test.rb +++ b/test/redis_client/subscriptions_test.rb @@ -89,5 +89,18 @@ def test_pubsub_with_disabled_reconnection refute_nil @redis.pubsub end end + + def test_pubsub_timeout_retry + assert_nil @subscription.call("SUBSCRIBE", "mychannel") + refute_nil @subscription.next_event # subscribed event + + assert_nil @subscription.next_event(0.01) + @redis.call("PUBLISH", "mychannel", "test") + 1.times.each do # rubocop:disable Lint/UselessTimes + # We use 1.times.each to change the stack depth. + # See https://github.com/redis-rb/redis-client/issues/221 + refute_nil @subscription.next_event + end + end end end