> On Oct. 25, 2012, 9:23 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java, lines 133-134 > > <https://reviews.apache.org/r/7730/diff/1/?file=179601#file179601line133> > > > > why are we synchronizing this? it's already protected by outgoingQueue, > > is this not enough? (it was only protected by outgoingQueue in 3.3 branch, > > why did it change?) > > Skye Wanderman-Milne wrote: > pendingQueue is not always protected by outgoingQueue (see > ClientCnxn.SendThread.readResponse, SendThread.cleanup). It's likely that > these functions are never called concurrently with doIO but I'll have to look > into it, and maybe we should document somewhere that pendingQueue should be > protected by outgoingQueue (since right now it's not). I'll also look into > what's going on in 3.3. > > Skye Wanderman-Milne wrote: > In 3.3 the other references to pendingQueue (readResponse, cleanup) are > synced on itself (and not outgoingQueue), so it looks like not syncing in > doIO was an oversight and we should keep the synchronized block.
Ok. In that case can you verify that there are no lock ordering issues that might cause a deadlock? - Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7730/#review12792 ----------------------------------------------------------- On Oct. 25, 2012, 10:51 p.m., Skye Wanderman-Milne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7730/ > ----------------------------------------------------------- > > (Updated Oct. 25, 2012, 10:51 p.m.) > > > Review request for zookeeper, Patrick Hunt and Ted Yu. > > > Description > ------- > > see ZOOKEEPER-1560 JIRA > > > This addresses bug ZOOKEEPER-1560. > https://issues.apache.org/jira/browse/ZOOKEEPER-1560 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java 70d8538 > > Diff: https://reviews.apache.org/r/7730/diff/ > > > Testing > ------- > > unit tests (including testLargeNodeData from ZOOKEEPER-1560 JIRA) > > > Thanks, > > Skye Wanderman-Milne > >
