[ https://issues.apache.org/jira/browse/ZOOKEEPER-59?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842248#action_12842248 ]
Flavio Paiva Junqueira commented on ZOOKEEPER-59: ------------------------------------------------- As I already mentioned above, I don't think it requires a test. Ben, could you please check my previous post to see if you agree? > Synchronized block in NIOServerCnxn > ----------------------------------- > > Key: ZOOKEEPER-59 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-59 > Project: Zookeeper > Issue Type: Bug > Components: server > Reporter: Flavio Paiva Junqueira > Assignee: Flavio Paiva Junqueira > Fix For: 3.3.0 > > Attachments: ZOOKEEPER-59.patch, ZOOKEEPER-59.patch > > > There are two synchronized blocks locking on different objects, and to me > they should be guarded by the same object. Here are the parts of the code I'm > talking about: > {noformat} > nioservercnxn.readrequ...@444 > ... > synchronized (this) { > outstandingRequests++; > // check throttling > if (zk.getInProcess() > factory.outstandingLimit) { > disableRecv(); > // following lines should not be needed since we are > already > // reading > // } else { > // enableRecv(); > } > } > {noformat} > {noformat} > nioservercnxn.sendrespo...@740 > ... > synchronized (this.factory) { > outstandingRequests--; > // check throttling > if (zk.getInProcess() < factory.outstandingLimit > || outstandingRequests < 1) { > sk.selector().wakeup(); > enableRecv(); > } > } > {noformat} > I think the second one is correct, and the first synchronized block should be > guarded by "this.factory". > This could be related to issue ZOOKEEPER-57, but I have no concrete > indication that this is the case so far. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.