Skip to content

Commit 3a4afda

Browse files
committed
continuing with issue 34388 fixing bug with connection being stuck on awaitUninterruptibly() as per fizzed#1
1 parent e130334 commit 3a4afda

File tree

1 file changed

+30
-21
lines changed

1 file changed

+30
-21
lines changed

src/main/java/com/cloudhopper/smpp/impl/DefaultSmppClient.java

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -292,28 +292,37 @@ protected Channel createConnectedChannel(String host, int port, long connectTime
292292
// attempt to connect to the remote system
293293
logger.info("Attempting to connect to remote system " + host + ":" + port);
294294
ChannelFuture connectFuture = this.clientBootstrap.connect(socketAddr);
295-
296-
// wait until the connection is made successfully
297-
// boolean timeout = !connectFuture.await(connectTimeoutMillis);
298-
// BAD: using .await(timeout)
299-
// see http://netty.io/3.9/api/org/jboss/netty/channel/ChannelFuture.html
300-
connectFuture.awaitUninterruptibly();
301-
//assert connectFuture.isDone();
302-
if (connectFuture.isDone()) {
303-
logger.info("Connected to remote system " + host + ":" + port);
304-
} else {
305-
logger.info("Uncompleted connection to remote system " + host + ":" + port);
306-
}
307295

308-
if (connectFuture.isCancelled()) {
309-
throw new InterruptedException("connectFuture cancelled by user");
310-
} else if (!connectFuture.isSuccess()) {
311-
if (connectFuture.getCause() instanceof org.jboss.netty.channel.ConnectTimeoutException) {
312-
throw new SmppChannelConnectTimeoutException("Unable to connect to host [" + host + "] and port [" + port + "] within " + connectTimeoutMillis + " ms", connectFuture.getCause());
313-
} else {
314-
throw new SmppChannelConnectException("Unable to connect to host [" + host + "] and port [" + port + "]: " + connectFuture.getCause().getMessage(), connectFuture.getCause());
315-
}
316-
}
296+
297+
// if (connectFuture.isDone()) {
298+
// logger.info("Connected to remote system " + host + ":" + port);
299+
// } else {
300+
// logger.info("Uncompleted connection to remote system " + host + ":" + port);
301+
// }
302+
303+
// Wait until the connection is made successfully.
304+
// According to the netty documentation it is bad to use .await(timeout). Instead
305+
// b.setOption("connectTimeoutMillis", 10000);
306+
// should be used. See http://netty.io/3.9/api/org/jboss/netty/channel/ChannelFuture.html
307+
// It turns out that under certain unknown circumstances the connect waits forever: https://github.com/twitter/cloudhopper-smpp/issues/117
308+
// That's why the future is canceled 1 second after the specified timeout.
309+
// This is a workaround and hopefully not needed after the switch to netty 4.
310+
if (!connectFuture.await(connectTimeoutMillis + 1000)) {
311+
logger.error("connectFuture did not finish in expected time! Try to cancel the connectFuture");
312+
boolean isCanceled = connectFuture.cancel();
313+
logger.error("connectFuture: isCanceled {} isDone {} isSuccess {}", isCanceled, connectFuture.isDone(), connectFuture.isSuccess());
314+
throw new SmppChannelConnectTimeoutException("Could not connect to the server within timeout");
315+
}
316+
317+
if (connectFuture.isCancelled()) {
318+
throw new InterruptedException("connectFuture cancelled by user");
319+
} else if (!connectFuture.isSuccess()) {
320+
if (connectFuture.getCause() instanceof org.jboss.netty.channel.ConnectTimeoutException) {
321+
throw new SmppChannelConnectTimeoutException("Unable to connect to host [" + host + "] and port [" + port + "] within " + connectTimeoutMillis + " ms", connectFuture.getCause());
322+
} else {
323+
throw new SmppChannelConnectException("Unable to connect to host [" + host + "] and port [" + port + "]: " + connectFuture.getCause().getMessage(), connectFuture.getCause());
324+
}
325+
}
317326

318327
logger.info("Successfully connected to remote system " + host + ":" + port);
319328
// if we get here, then we were able to connect and get a channel

0 commit comments

Comments
 (0)