[
https://issues.apache.org/jira/browse/ZOOKEEPER-1029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15050434#comment-15050434
]
Flavio Junqueira commented on ZOOKEEPER-1029:
---------------------------------------------
[~rgs] thanks for the review and comments.
# Previously, the return value was void, so if there was no adaptor, then we'd
silently move on. By returning 0, I'm essentially keeping the same behavior and
saying that it is OK to make progress. The case this patch is covering is
focused on the one that it does try to acquire the lock, but the lock operation
fails. Also, if you want to make this change here, then we will need to fix it
in the 3.4 branch as well. With this patch, I'm just porting what we've done
for the 3.4 branch so far.
# I don't think it is ok to move the while loop out of the if block because we
aren't initializing tmp_list. Even if we were, there isn't going to be any
iteration of the while loop because there is no head.
# It sounds ok to make this change, but I don't see much advantage or any
correction of behavior. If we don't call the if block as you define it, then
we'll have a_list.completion = NULL and a_list.next = NULL, so there will be no
iteration of the subsequent while loop.
> C client bug in zookeeper_init (if bad hostname is given)
> ---------------------------------------------------------
>
> Key: ZOOKEEPER-1029
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1029
> Project: ZooKeeper
> Issue Type: Bug
> Components: c client
> Affects Versions: 3.3.2, 3.4.6, 3.5.0
> Reporter: Dheeraj Agrawal
> Assignee: Flavio Junqueira
> Priority: Blocker
> Fix For: 3.4.7, 3.5.2, 3.6.0
>
> Attachments: ZOOKEEPER-1029-3.4.patch, ZOOKEEPER-1029-3.4.patch,
> ZOOKEEPER-1029-3.4.patch, ZOOKEEPER-1029-3.4.patch, ZOOKEEPER-1029-3.4.patch,
> ZOOKEEPER-1029-3.4.patch, ZOOKEEPER-1029-3.5.patch
>
>
> If you give invalid hostname to zookeeper_init method, it's not able to
> resolve it, and it tries to do the cleanup (free buffer/completion lists/etc)
> . The adaptor_init() is not called for this code path, so the lock,cond
> variables (for adaptor, completion lists) are not initialized.
> As part of the cleanup it's trying to clean up some buffers and acquires
> locks and unlocks (where the locks have not yet been initialized, so
> unlocking fails)
> lock_completion_list(&zh->sent_requests); - pthread_mutex/cond not
> initialized
> tmp_list = zh->sent_requests;
> zh->sent_requests.head = 0;
> zh->sent_requests.last = 0;
> unlock_completion_list(&zh->sent_requests); trying to broadcast here
> on uninitialized cond
> It should do error checking to see if locking succeeds before unlocking it.
> If Locking fails, then appropriate error handling has to be done.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
