Mahadev konar commented on ZOOKEEPER-318:

your 2 cents are well received :) .. 
ill just explain where I come from. From my perspective, a multithreaded c code 
is the most difficult to debug (after ofcourse assembly language :) ) ... So, I 
get really uncomfortable removing locks from the c code (maybe i am a little 
paranoid). I would rather add locks and make it MT safe and be future problem 
resilient. But ofcourse thats my stand on it. I am not too biased on my opinion 
and would not like to reject any contributions on my own personal opinion. If 
you do want to contribute the patch as it is, Ill go ahead with that.

> remove locking in zk_hashtable.c or add locking in collect_keys()
> -----------------------------------------------------------------
>                 Key: ZOOKEEPER-318
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-318
>             Project: Zookeeper
>          Issue Type: Bug
>          Components: c client
>    Affects Versions: 3.0.0, 3.0.1, 3.1.0
>            Reporter: Chris Darroch
>            Assignee: Chris Darroch
>             Fix For: 3.2.0, 4.0.0
>         Attachments: ZOOKEEPER-318.patch
> From a review of zk_hashtable.c it appears to me that all functions which 
> manipulate the hashtables are called from the IO thread, and therefore any 
> need for locking is obviated.
> If I'm wrong about that, then I think at a minimum collect_keys() should 
> acquire a lock in the same manner as collect_session_watchers().  Both 
> iterate over hashtable contents (in the latter case using copy_table()).
> However, from what I can see, the only function (besides the init/destroy 
> functions used when creating a zhandle_t) called from the completion thread 
> is deliverWatchers(), which simply iterates over a "delivery" list created 
> from the hashtables by collectWatchers().  The activateWatcher() function 
> contains comments which describe it being called by the completion thread, 
> but in fact it is called by the IO thread in zookeeper_process().
> I believe all calls to collectWatchers(), activateWatcher(), and 
> collect_keys() are made by the IO thread in zookeeper_interest(), 
> zookeeper_process(), check_events(), send_set_watches(), and handle_error().  
> Note that queue_session_event() is aliased as PROCESS_SESSION_EVENT, but 
> appears only in handle_error() and check_events().
> Also note that handle_error() is called only in zookeeper_process() and 
> handle_socket_error_msg(), which is used only by the IO thread, so far as I 
> can see.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to