Skip to content
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

ZOOKEEPER-2687:Deadlock while shutting down the Leader server. #176

Closed
wants to merge 2 commits into from
Closed

ZOOKEEPER-2687:Deadlock while shutting down the Leader server. #176

wants to merge 2 commits into from

Conversation

arshadmohammad
Copy link
Contributor

Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380

@arshadmohammad
Copy link
Contributor Author

There is not test case, as the change is very simple and understandable and the test will complicate the overall fix.

@afine
Copy link
Contributor

afine commented Feb 14, 2017

+1 lgtm

looked through he code and i did not see any other places where this particular deadlock occurs

@@ -590,8 +590,8 @@ void lead() throws IOException, InterruptedException {

// check leader running status
if (!this.isRunning()) {
shutdown("Unexpected internal error");
return;
shutdownMessage = "Unexpected internal error";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @arshadmohammad. Yes, this is another code where it can leads to deadlock similar to ZK-2380.

As we know, shutdownMessage is acting as flag. Could you please add comments to make this clear,
// set shutdown flag
shutdownMessage = "Unexpected internal error";`

@rakeshadr
Copy link
Contributor

+1 LGTM, I will merge this to branch-3.5 and master shortly.

@asfgit asfgit closed this in fd211a5 Feb 15, 2017
asfgit pushed a commit that referenced this pull request Feb 15, 2017
Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380

Author: Mohammad Arshad <[email protected]>

Reviewers: Abraham Fine <[email protected]>, Rakesh Radhakrishnan <[email protected]>

Closes #176 from arshadmohammad/ZOOKEEPER-2687 and squashes the following commits:

7551f5c [Mohammad Arshad] Fixed Rakesh's comments
1e3ed70 [Mohammad Arshad] ZOOKEEPER-2687:Deadlock while shutting down the Leader server.

(cherry picked from commit fd211a5)
Signed-off-by: Rakesh Radhakrishnan <[email protected]>
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380

Author: Mohammad Arshad <[email protected]>

Reviewers: Abraham Fine <[email protected]>, Rakesh Radhakrishnan <[email protected]>

Closes apache#176 from arshadmohammad/ZOOKEEPER-2687 and squashes the following commits:

7551f5c [Mohammad Arshad] Fixed Rakesh's comments
1e3ed70 [Mohammad Arshad] ZOOKEEPER-2687:Deadlock while shutting down the Leader server.
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380

Author: Mohammad Arshad <[email protected]>

Reviewers: Abraham Fine <[email protected]>, Rakesh Radhakrishnan <[email protected]>

Closes apache#176 from arshadmohammad/ZOOKEEPER-2687 and squashes the following commits:

7551f5c [Mohammad Arshad] Fixed Rakesh's comments
1e3ed70 [Mohammad Arshad] ZOOKEEPER-2687:Deadlock while shutting down the Leader server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants