You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/03/08 16:41:27 UTC
[GitHub] [activemq] cshannon opened a new pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
cshannon opened a new pull request #620:
URL: https://github.com/apache/activemq/pull/620
This is needed to prevent concurrent access to the SSLEngine during
initialization. This is a regression from when auto+nio+ssl was added.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-792893731
I guess the same should be applied to AMQ SSL test, I will tackle this if required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-792893168
Thanks, I'm testing on my machine as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-793538738
I'm testing on the PR now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-793755871
The main thing I was looking at was StompNIOSSLTest#testDisconnectDoesNotDeadlockBroker was breaking in CI and also locally for me and this PR fixed that at least. That other test you reference has always been flaky even before I was working on the auto transport stuff years ago.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-793538069
FYI, when I do:
```
mvn clean test -Dtest=Stomp11NIOSSLTest
```
I do the same again:
```
[ERROR] Failures:
[ERROR] Stomp11NIOSSLTest>Stomp11Test.testDurableSubAndUnSubOnTwoTopics:939
[INFO]
[ERROR] Tests run: 31, Failures: 1, Errors: 0, Skipped: 0
```
It's what I meant when the test is flaky.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-794075527
I ran `activemq-stomp` build several times without failure. So, it seems your change improves the overall situation. I'm doing a set of new run and I will merge.
Thanks !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-793767278
OK, so it's what I thought: it's not the same thing, and the build randomly fail (including on Jenkins). I will do a new test on those tests, eventually excluding them (or isolating in a profile).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre edited a comment on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre edited a comment on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-793538069
FYI, when I do:
```
mvn clean test -Dtest=Stomp11NIOSSLTest
```
it works.
I do the same again:
```
[ERROR] Failures:
[ERROR] Stomp11NIOSSLTest>Stomp11Test.testDurableSubAndUnSubOnTwoTopics:939
[INFO]
[ERROR] Tests run: 31, Failures: 1, Errors: 0, Skipped: 0
```
It's what I meant when the test is flaky.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #620:
URL: https://github.com/apache/activemq/pull/620#issuecomment-794083837
Thanks for checking that. The change seems like it should prevent any more of those errors by preventing concurrent access to the SSL engine. Also generally there won't be any contention after init so there should be very minimal overhead with uncontended locking so I wouldn't expect any noticeable performance hit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre merged pull request #620: AMQ-8169: Synchronize on serviceRead inside NIOSSLTransport
Posted by GitBox <gi...@apache.org>.
jbonofre merged pull request #620:
URL: https://github.com/apache/activemq/pull/620
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org