search

shinberg / cpp-hiredis-cluster

c++ cluster wrapper for hiredis with async and unix sockets features
BSD 3-Clause "New" or "Revised" License
65 stars 26 forks source link

ThreadedPool can not make Cluster thread-safe #15

Closed jinq0123 closed 7 years ago

jinq0123 commented 7 years ago

ThreadedPool can not make Cluster thread-safe because neither redisContext nor redisAsyncContext is thread-safe.

From hiredis readme:

    Note: A redisContext is not thread-safe.
    Note: A redisAsyncContext is not thread-safe.

So threadedpool example is wrong. It executes 1000 commandThread() which calls redisAppendFormattedCommand() without mutex lock.

shinberg commented 7 years ago

Hi, Let me describe me why it's completely thread safe.

redisContext is not thread safe i know. But this does not mean that you can't use one redisContext instance in several threads. You can, but carefuly - If the only one thread executes commands on particular redisContext instance in time - it's ok! Trust me, I've inspected the sources of hiredis library.

How thread safety is achived? - there is the class ThreadedPool which purpose is to ensure that only one thread can execute commands on particular redisContext instance in particular moment. Thread initially gets one redisContext from pool (at this time connection is deleted from pool), executes the command, and lets it back to the pool. So there's no threading error. Thank you for your post.