You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/12/15 15:30:38 UTC
svn commit: r1720176 - in /tomcat/trunk:
java/org/apache/coyote/http2/Http2UpgradeHandler.java
webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org