-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Thread safety issue with PubSub and hiredis #208
Comments
From my naive understanding, it seems flushing the write buffer inside redis-client/hiredis-client/lib/redis_client/hiredis_connection.rb Lines 109 to 129 in e8aff08
Removing the write buffer flush from |
Nice catch. |
I've been digging into some Ruby crashes we've been seeing with ActionCable in our test suite, and I think I've uncovered a thread safety issue when
HiredisConnection
is used with#pubsub
.Minimal reproduction:
I understand using multiple threads here is supposed to be safe, as PubSub
#call_v
only writes to the socket, and#next_event
only reads from the socket (see also redis/redis-rb#1131).Without hiredis-client, this runs indefinitely without errors.
With hiredis-client, it either fails with a weird connection error:
Or crashes Ruby with a seg fault:
Ruby crash dump
With Ruby 3.4.0-preview1 compiled with ASAN, ASAN detects a heap-use-after-free.
Full ASAN report
T2 in the ASAN report is the
reader
Ruby thread that's calling#next_event
:This thread should just be reading, but it ends up accessing the write buffer, as hiredis_read_internal also drains the write buffer.
The write buffer had already been freed by the main Ruby thread that's writing
subscribe
andunsubscribe
commands:The text was updated successfully, but these errors were encountered: