You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Patrick Hunt (JIRA)" <ji...@apache.org> on 2008/07/01 00:08:45 UTC
[jira] Created: (ZOOKEEPER-63) Race condition in client close()
operation
Race condition in client close() operation
------------------------------------------
Key: ZOOKEEPER-63
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
Project: Zookeeper
Issue Type: Bug
Components: java client
Reporter: Patrick Hunt
Assignee: Benjamin Reed
There is a race condition in the java close operation on ZooKeeper.java.
Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
james strachan updated ZOOKEEPER-63:
------------------------------------
Attachment: patch_ZOOKEEPER-63.patch
This patch avoids the close() method blocking forever. It waits just once, up to the closeTimeout so if the socket is blocked or some other strangeness is going on, the calling thread will only wait up to the timeout (which defaults to 2 seconds).
BTW this patch fixes the hang I was having in the test case to ZOOKEEPER-78
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Assignee: james strachan (was: Benjamin Reed)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt reassigned ZOOKEEPER-63:
-------------------------------------
Assignee: Patrick Hunt (was: james strachan)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12620386#action_12620386 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
SessionTest testReuse test is intermittently failing seeming due to this issue.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Attachment: (was: patch_ZOOKEEPER-63.patch)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Stu Hood (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stu Hood updated ZOOKEEPER-63:
------------------------------
Attachment: client-test-fail.diff
I believe this bug will eventually cause a client to run out of file handles if they connect and disconnect often enough, so it is a pretty serious issue.
The attached modification to the ClientTest testcase causes it to fail by running out of file handles, and we are seeing this behaviour in our application.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-63:
-----------------------------------
Hadoop Flags: [Reviewed]
+1 Great work Pat. I'll commit.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616426#action_12616426 ]
james strachan commented on ZOOKEEPER-63:
-----------------------------------------
So this patch does not attempt to fix the race condition problem, apologies if I gave that impression :)
What it does do though is act as a workaround so that if a client is not able to properly send a disconnect packet to the server for *any reason at all* such as
* a hung socket (which can be quite common)
* no servers available
* a race condition in the ZK client code of some kind (which we definitely have now)
to not hang the client application forever - as its trying to close and shut down anyway :). So its a side benefit that it acts as a band aid until someone fixes all the possible race conditions and potential socket hangs.
Let me put it another way. Given that the client is closing; is it really correct to leave it potentially hanging around forever just because it cannot be sure if the disconnect packet was received and properly processed by the server? If the socket is dead before the call to close(), is it really correct to block until a connection can be re-established, just so it can be properly closed - when the code will effectively close the hung socket without sending a disconnect packet anyway :) ?
The server has to detect and timeout failed sessions; whether it receives an explicit disconnect packet or not (as a process could just hang). So do we really need to be super strict on the client side, forcing clients to block when trying to shut down if they can't do so cleanly within some time period?
I totally agree that we should fix the race condition though :). I just wanted a work around to avoid my ZK test cases hanging forever due to the race condition :)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-63:
-----------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
Committed revision 689668.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Fix Version/s: 3.0.0
Affects Version/s: 3.0.0
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Affects Versions: 3.0.0
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Fix For: 3.0.0
>
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626541#action_12626541 ]
Hudson commented on ZOOKEEPER-63:
---------------------------------
Integrated in ZooKeeper-trunk #67 (See [http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/67/])
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616219#action_12616219 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
Correct me if I'm wrong but I don't believe this "fixes" anything, it merely ignores the problem.
1) there is still the stated race condition
2) what is causing the underlying hang? this should not be happening and needs to be addressed directly, may even be related to item 1
3) if the timeout is reached a warning should be issued in the log
My current vote is -1
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12619041#action_12619041 ]
Benjamin Reed commented on ZOOKEEPER-63:
----------------------------------------
-1 The submitRequest logic should not be duplicated. (It will cause maintenance problems futher down the line.) The original submitRequest should be changed to call submitRequest(h, request, response, watchRegistration, 0)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625880#action_12625880 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
Stu, it turned out there were two problem here - the close race but also the client connection close was not closing the selector which was causing resources to leak. This patch fixes both.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12620396#action_12620396 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
Assigning to myself - I've cleaned up the tests as part of ZOOKEEPER-111 (this was blocking progress on this issue). I'll dig into this issue now.
Looking at the code for zk close it seems to me that the issue is related to how the client send/event threads manage state. In particular these threads look "out/up" at the parent (zookeeper.state) rather than managing state internally and having operations for code (zk client) to make changes to that state.
There is a race condition in the close where sendthread thinks that the connection is still open (not closed) and retries the server connection rather than shutting down.
I will probably have to have significant changes to how send/event threads are managed - they need to manage their own internal state and take updates from zookeeper client.... we'll see.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-63:
-----------------------------------
Status: Patch Available (was: Open)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Stu Hood (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625619#action_12625619 ]
Stu Hood commented on ZOOKEEPER-63:
-----------------------------------
We had a non-daemon thread that needed to have its own ephemerals, so we let it create its own connection/session. During one of our unit tests, the thread gets created a few hundred times, and we noticed the issue. In the real world, the work the thread will be performing will take a few order of magnitude longer, but we still would rather not leak any file handles =)
We've modified our code to maintain a single connection for that thread: you're right about the session management. Thanks Patrick!
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-63:
-----------------------------------
Attachment: patch_ZOOKEEPER-63.patch
I've fixed the patch to not duplicate the logic in submitRequest
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch, patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Attachment: ZOOKEEPER-63.patch
This patch:
1) fixes the close race condition
2) fixes a resource issue where close operation was not causing the selector to be closed (leak)
3) updated tests to look for resource leak testClientCleanup (thanks Stu)
4) updated tests to better verify close operation successful
The only real change is in ClientCnxn - setting a "closing" latch that keeps the send thread from re-establishing a broken connection -- but allows the client to continue communication with server (necessary to send the session disconnect message).
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-63:
-----------------------------------
Attachment: (was: patch_ZOOKEEPER-63.patch)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616136#action_12616136 ]
james strachan commented on ZOOKEEPER-63:
-----------------------------------------
I wonder if I've seen this too - I can reliably get a hung test when trying to close a client (though the server is still up at the point if the hang).
I'm thinking the close() method should not wait() forever on the disconnect packet, just a closeTimeout length - say a few seconds. Afterall blocking and forcing a reconnect just to redeliver the disconnect packet seems a bit silly - when the server has to deal with clients which just have their sockets fail anyway :)
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
james strachan updated ZOOKEEPER-63:
------------------------------------
Status: Patch Available (was: Open)
about to attach a patch
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Stu Hood (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12631021#action_12631021 ]
Stu Hood commented on ZOOKEEPER-63:
-----------------------------------
Great work Patrick! Thanks a bunch for looking into it.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12631506#action_12631506 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
Thank you for taking the time to report the issue. Regards.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12619117#action_12619117 ]
Benjamin Reed commented on ZOOKEEPER-63:
----------------------------------------
You are correct. The patch itself is good, but it does not fix the stated issue.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Status: Open (was: Patch Available)
I agree with your comments, in addition the patch should address:
1) there is still the stated race condition
2) what is causing the underlying hang? this should not be happening and needs to be addressed directly, may even be related to item 1
3) if the timeout is reached a warning should be issued in the log
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616151#action_12616151 ]
james strachan commented on ZOOKEEPER-63:
-----------------------------------------
BTW here's the hang I seem to be able to get quite easily using the test case WriteLockTest in the ZOOKEEPER-78 patch (you need to set workAroundClosingLastZNodeFails to false to make it hang)
{code}
[junit] "main" prio=5 tid=0x01001710 nid=0xb0801000 in
Object.wait() [0xb07ff000..0xb0800148]
[junit] at java.lang.Object.wait(Native Method)
[junit] - waiting on <0x096105e0> (a
org.apache.zookeeper.ClientCnxn$Packet)
[junit] at java.lang.Object.wait(Object.java:474)
[junit] at
org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:822)
[junit] - locked <0x096105e0> (a org.apache.zookeeper.ClientCnxn$Packet)
[junit] at org.apache.zookeeper.ZooKeeper.close(ZooKeeper.java:329)
[junit] - locked <0x0bd54108> (a org.apache.zookeeper.ZooKeeper)
[junit] at
org.apache.zookeeper.protocols.ZooKeeperFacade.close(ZooKeeperFacade.java:99)
[junit] at
org.apache.zookeeper.protocols.WriteLockTest.tearDown(WriteLockTest.java:146)
[junit] at junit.framework.TestCase.runBare(TestCase.java:140)
[junit] at junit.framework.TestResult$1.protect(TestResult.java:110)
[junit] at junit.framework.TestResult.runProtected(TestResult.java:128)
[junit] at junit.framework.TestResult.run(TestResult.java:113)
[junit] at junit.framework.TestCase.run(TestCase.java:124)
[junit] at junit.framework.TestSuite.runTest(TestSuite.java:232)
[junit] at junit.framework.TestSuite.run(TestSuite.java:227)
[junit] at
org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:81)
[junit] at
junit.framework.JUnit4TestAdapter.run(JUnit4TestAdapter.java:36)
[junit] at
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:421)
[junit] at
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:912)
[junit] at
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:766)
{code}
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Benjamin Reed
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Status: Patch Available (was: Open)
Please review ZOOKEEPER-63.patch file and commit.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625561#action_12625561 ]
Patrick Hunt commented on ZOOKEEPER-63:
---------------------------------------
Stu, thanks for the test! I'll update ClientTest to include this, pretty ugly.
Just wondering, why are you frequently connecting/disconnecting a client? Could you establish a connection and keep it open for a long(er) period of time? Session establishment by a particular client is an expense that needs to be paid by all servers in a quorum, so minimizing will generally improve your overall zk cluster performance.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: Patrick Hunt
> Attachments: client-test-fail.diff, patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-63) Race condition in client close()
operation
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-63?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-63:
----------------------------------
Status: Open (was: Patch Available)
-1 on the patch, still have the stated unresolved issues with this change:
1) there is still the stated race condition
2) what is causing the underlying hang? this should not be happening and needs to be addressed directly, may even be related to item 1
3) if the timeout is reached a warning should be issued in the log
I suspect we should split this patch out to it's own jira and make it blocked by this, perhaps that would make it more clear.
> Race condition in client close() operation
> ------------------------------------------
>
> Key: ZOOKEEPER-63
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-63
> Project: Zookeeper
> Issue Type: Bug
> Components: java client
> Reporter: Patrick Hunt
> Assignee: james strachan
> Attachments: patch_ZOOKEEPER-63.patch
>
>
> There is a race condition in the java close operation on ZooKeeper.java.
> Client is sending a disconnect request to the server. Server will close any open connections with the client when it receives this. If the client has not yet shutdown it's subthreads (event/send threads for example) these threads may consider the condition an error. We see this alot in the tests where the clients output error logs because they are unaware that a disconnection has been requested by the client.
> Ben mentioned: perhaps we just have to change state to closed (on client) before sending disconnect request.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.