Summary: | Upgrade processor is put in recycledProcessors stack of Http11NioProtocol.Http11ConnectionHandler | ||
---|---|---|---|
Product: | Tomcat 8 | Reporter: | Alexander Malyshev <anmalyshev> |
Component: | Connectors | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 8.0.33 | ||
Target Milestone: | ---- | ||
Hardware: | PC | ||
OS: | Linux |
Description
Alexander Malyshev
2016-04-08 14:23:10 UTC
This need to be verified, but I do notice some upgraded processors going into this code path when running the websockets portion of the testsuite. I added an extra check based on the upgraded flag. Does that mean that you have a fix for this issue? Looking at the code it's not clean if it's enough to just skip recycling such Upgrade connectors or it's also needed to execute the code similar to the one executed in normal scenario: UpgradeToken upgradeToken = processor.getUpgradeToken(); HttpUpgradeHandler httpUpgradeHandler = upgradeToken.getHttpUpgradeHandler(); InstanceManager instanceManager = upgradeToken.getInstanceManager(); if (instanceManager == null) { httpUpgradeHandler.destroy(); } else { ClassLoader oldCL = upgradeToken.getContextBind().bind(false, null); try { httpUpgradeHandler.destroy(); instanceManager.destroyInstance(httpUpgradeHandler); } finally { upgradeToken.getContextBind().unbind(false, oldCL); } } If you have a patch I can apply it, build tomcat and test the fix. The change you mentioned is for trunk. I'm using 8.0.33, and it's quite different. For example it does not have implemented release() method in AbstractProtocol class. I applied similar change to Http11NioProtocol class and will retest. But if you can provide actual diff for 8.0 (which will be eventually released) - it would be much better as I'll be able to make sure this change is actually fixing an issue. Even 7 had the issue when adding a debug in the right release method and using the websocket portion of the testsuite. Oops. Remy's patches should have fixed this. The fix will be in 9M5, 8.5.1, 8.0.34 and 7.0.70. Applied the patch and tested - issue is fixed. |