You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by cshannon <gi...@git.apache.org> on 2018/03/02 16:39:34 UTC
[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...
GitHub user cshannon opened a pull request:
https://github.com/apache/activemq-artemis/pull/1925
ARTEMIS-1727 - Make sure transport is stopped on failed OpenWire
connection
To prevent a socket from hanging open by a bad client the broker should
make sure to stop the transport if a connection attempt fails by an
OpenWire client
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cshannon/activemq-artemis ARTEMIS-1727
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1925.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1925
----
commit 8de2fccc883aa3a532e540e5e700e2c668e4e289
Author: Christopher L. Shannon (cshannon) <ch...@...>
Date: 2018-03-02T16:38:07Z
ARTEMIS-1727 - Make sure transport is stopped on failed OpenWire
connection
To prevent a socket from hanging open by a bad client the broker should
make sure to stop the transport if a connection attempt fails by an
OpenWire client
----
---
[GitHub] activemq-artemis issue #1925: ARTEMIS-1727 - Make sure transport is stopped ...
Posted by cshannon <gi...@git.apache.org>.
Github user cshannon commented on the issue:
https://github.com/apache/activemq-artemis/pull/1925
@tabish121 - PR has been updated to use the broker's scheduled pool
---
[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...
Posted by cshannon <gi...@git.apache.org>.
Github user cshannon commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1925#discussion_r171921837
--- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java ---
@@ -641,13 +647,37 @@ public void fail(ActiveMQException me, String message) {
}
}
try {
- protocolManager.removeConnection(this.getConnectionInfo(), me);
+ if (this.getConnectionInfo() != null) {
+ protocolManager.removeConnection(this.getConnectionInfo(), me);
+ }
} catch (InvalidClientIDException e) {
ActiveMQServerLogger.LOGGER.warn("Couldn't close connection because invalid clientID", e);
}
shutdown(true);
}
+ private void delayedStop(final int waitTime, final String reason, Throwable cause) {
+ if (waitTime > 0) {
+ try {
+ server.getExecutorFactory().getExecutor().execute(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(waitTime);
--- End diff --
Agreed, I will make that change and use a scheduled executor instead.
---
[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1925#discussion_r171918979
--- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java ---
@@ -641,13 +647,37 @@ public void fail(ActiveMQException me, String message) {
}
}
try {
- protocolManager.removeConnection(this.getConnectionInfo(), me);
+ if (this.getConnectionInfo() != null) {
+ protocolManager.removeConnection(this.getConnectionInfo(), me);
+ }
} catch (InvalidClientIDException e) {
ActiveMQServerLogger.LOGGER.warn("Couldn't close connection because invalid clientID", e);
}
shutdown(true);
}
+ private void delayedStop(final int waitTime, final String reason, Throwable cause) {
+ if (waitTime > 0) {
+ try {
+ server.getExecutorFactory().getExecutor().execute(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(waitTime);
--- End diff --
Sleeping here seems like it might have a ripple on the other resources using the executor, maybe use a scheduled executor from the pool in ActiveMQServer ?
---
[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...
Posted by cshannon <gi...@git.apache.org>.
Github user cshannon commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1925#discussion_r171924792
--- Diff: artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java ---
@@ -641,13 +647,37 @@ public void fail(ActiveMQException me, String message) {
}
}
try {
- protocolManager.removeConnection(this.getConnectionInfo(), me);
+ if (this.getConnectionInfo() != null) {
+ protocolManager.removeConnection(this.getConnectionInfo(), me);
+ }
} catch (InvalidClientIDException e) {
ActiveMQServerLogger.LOGGER.warn("Couldn't close connection because invalid clientID", e);
}
shutdown(true);
}
+ private void delayedStop(final int waitTime, final String reason, Throwable cause) {
+ if (waitTime > 0) {
+ try {
+ server.getExecutorFactory().getExecutor().execute(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(waitTime);
--- End diff --
I won't have time today but I will fix my patch and push it up first thing monday morning.
---
[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1925
---