Community
Participate
Working Groups
Build Identifier: Jetty distribution 7.2.0.v20101020 In org.eclipse.jetty.client.HttpDestination calls to startNewConnection() are being made from within synchronized blocks. During the startNewConnection() call, all HttpConnections for that HttpDestination are blocked from being returned to the HttpDestination and calls to HttpDestination.getIdleConnection() are also blocked. Since creating a new socket connection can be very slow and startNewConnection() doesn't need to be called from within a synchronized block, calls to startNewConnection() should be moved outside of synchronized blocks. This is safe because: - startNewConnection itself performs necessary synchronization to update _pendingConnections. - The call to connector.startConnection() doesn't need to be synchronized. - The call to connector.startConnection() will result in one of: onConnectionFailed(), onException(), or onNewConnection() being called. Each of which handles necessary synchronization. I'll be attaching a patch with a proposed fix. Reproducible: Always Steps to Reproduce: 1. Make multiple simultaneous requests to the same web server. 2. Force the server to be slow in accepting one of the connections. 3. Observe that getIdleConnection() and returnConnection() will both block while the slow connection is being made.
Created attachment 192045 [details] Patch for a proposed fix.
Created attachment 192046 [details] Different diff format for proposed patch I'm not sure about the diff format of my initial patch file (what's up with the exclamation marks). I created it using "diff -crB" based on some website's advice. This new file is from an svn diff of the change I'm proposing (I've got a copy of the jetty source in a local subversion repo). If neither of these versions of the patch are acceptable, please let me know the proper way to provide a patch.
Simone, can you review the state of this issue. It may be that the changes I made to revert to an async connect may prevent the blocking anyway?
We have enhanced the connect functionality to support blocking style and non blocking style with the SelectConnector, and the default is blocking. The SocketConnector can only do blocking connects. So I am reviewing the patch as we should really attempt to connect outside sync blocks.
Patch applied with small changes only to variable names.
