Author: markt Date: Tue Dec 15 14:30:37 2015 New Revision: 1720176 URL: http://svn.apache.org/viewvc?rev=1720176&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58659 Refactor to avoid a deadlock caused by different sections of code obtaining the same locks in a different order.
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1720176&r1=1720175&r2=1720176&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Tue Dec 15 14:30:37 2015 @@ -733,24 +733,35 @@ public class Http2UpgradeHandler extends + @SuppressWarnings("sync-override") // notifyAll() needs to be outside sync + // to avoid deadlock @Override - protected synchronized void incrementWindowSize(int increment) throws Http2Exception { - long windowSize = getWindowSize(); - if (windowSize < 1 && windowSize + increment > 0) { - releaseBackLog((int) (windowSize +increment)); - } - super.incrementWindowSize(increment); - } + protected void incrementWindowSize(int increment) throws Http2Exception { + Set<AbstractStream> streamsToNotify = null; + synchronized (this) { + long windowSize = getWindowSize(); + if (windowSize < 1 && windowSize + increment > 0) { + streamsToNotify = releaseBackLog((int) (windowSize +increment)); + } + super.incrementWindowSize(increment); + } - private synchronized void releaseBackLog(int increment) { - if (backLogSize < increment) { - // Can clear the whole backlog - for (AbstractStream stream : backLogStreams.keySet()) { + if (streamsToNotify != null) { + for (AbstractStream stream : streamsToNotify) { synchronized (stream) { stream.notifyAll(); } } + } + } + + + private synchronized Set<AbstractStream> releaseBackLog(int increment) { + Set<AbstractStream> result = new HashSet<>(); + if (backLogSize < increment) { + // Can clear the whole backlog + result.addAll(backLogStreams.keySet()); backLogStreams.clear(); backLogSize = 0; } else { @@ -762,12 +773,11 @@ public class Http2UpgradeHandler extends int allocation = entry.getValue()[1]; if (allocation > 0) { backLogSize -= allocation; - synchronized (entry.getKey()) { - entry.getKey().doNotifyAll(); - } + result.add(entry.getKey()); } } } + return result; } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1720176&r1=1720175&r2=1720176&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Tue Dec 15 14:30:37 2015 @@ -164,6 +164,10 @@ <fix> Fix NIO connector renegotiation. (remm) </fix> + <fix> + <bug>58659</bug>: Fix a potential deadlock during HTTP/2 processing when + the connection window size is limited. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org