[JAVA-1575] Remove unnecessary synchronization in NettyStream Created: 28/Nov/14  Updated: 31/Mar/15  Resolved: 08/Dec/14

Status: Closed
Project: Java Driver
Component/s: Connection Management
Affects Version/s: None
Fix Version/s: 3.0.0

Type: Task Priority: Major - P3
Reporter: Ross Lawley Assignee: Jeffrey Yemin
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

NettyStream has been observed getting into deadlocks when multiple items call stream.writeAsync - notably ParallelCollectionScanOperationSpecification.should visit all documents asynchronously would consistently lock.

I removed the synchronization requirements round it the readAsync and writeAsync methods and but the new approach code needs reviewing.



 Comments   
Comment by Jeffrey Yemin [ 31/Mar/15 ]

Closing all resolved 3.0.0 issues, as 3.0.0 has been tagged and released.

Comment by Githook User [ 30/Jan/15 ]

Author:

{u'username': u'jyemin', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

Message: Added open/openAsync methods to Stream interface to make implementation simpler. Before this, implementations
either had to open in the constructor or in write/writeAsync.
Also removed unnecessary synchronization in NettyStream that made it prone to deadlock. The Stream client
is responsible for ensuring that there is at most one call to read/readAsync and one to write/writeAsync going on
concurrently, so Stream implementations do not have to do their own synchronization.

JAVA-1584, JAVA-1575
Branch: master
https://github.com/mongodb/mongo-java-driver/commit/b9ff37e15e86a7aba11812a2183283fb165c57ab

Comment by Jeffrey Yemin [ 10/Dec/14 ]

Author:

{u'username': u'jyemin', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

Message: NettyStream does need some synchronization between threads calling readAsync and Netty threads calling methods in InboundBufferHandler class

Branch: 3.0.x
https://github.com/mongodb/mongo-java-driver/commit/10283244d8cee7e173df6d98621a14e085a66c03

Comment by Githook User [ 04/Dec/14 ]

Author:

{u'username': u'jyemin', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

Message: Added open/openAsync methods to Stream interface to make implementation simpler. Before this, implementations
either had to open in the constructor or in write/writeAsync.
Also removed unnecessary synchronization in NettyStream that made it prone to deadlock. The Stream client
is responsible for ensuring that there is at most one call to read/readAsync and one to write/writeAsync going on
concurrently, so Stream implementations do not have to do their own synchronization.

JAVA-1584, JAVA-1575
Branch: 3.0.x
https://github.com/mongodb/mongo-java-driver/commit/b9ff37e15e86a7aba11812a2183283fb165c57ab

Comment by Jeffrey Yemin [ 04/Dec/14 ]

I see the deadlock, and it's in our own code, not Netty.

The problem is that you have one NIO thread with this stack

NettyStream@2.writeAsync
// chain of callback
NettyStream@1.readAsync

and another NIO thread with this stack:

NettyStream@1.writeAsync
// chain of callback
NettyStream@2.readAsync

If both readAsync and writeAsync are synchronized, that spells deadlock.

So these methods should not be synchronized at all, and it's up to the caller, InternalStreamConnection, to ensure proper synchronization, which I believe it already does.

Generated at Thu Feb 08 08:54:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.