Skip to content
This repository has been archived by the owner on Sep 18, 2021. It is now read-only.

Threads get stuck at DefaultSmppSession.sendRequestPd:530 #114

Open
mthyman opened this issue Sep 22, 2015 · 5 comments
Open

Threads get stuck at DefaultSmppSession.sendRequestPd:530 #114

mthyman opened this issue Sep 22, 2015 · 5 comments

Comments

@mthyman
Copy link

mthyman commented Sep 22, 2015

We have noticed that when the network between client and server or threads get stuck on line 530 in DefaultSmppSession. The code is:

// write the pdu out & wait timeout amount of time
ChannelFuture channelFuture = this.channel.write(buffer).await();

I think that the fix should be something like this:

// write the pdu out & wait timeout amount of time
ChannelFuture channelFuture = this.channel.write(buffer).await(timeoutMillis);

This is on version 5.0.7 and 5.0.8

@jjlauer
Copy link
Contributor

jjlauer commented Sep 22, 2015

You probably are onto something. Gladly will accept a PR that makes the change.

@mthyman
Copy link
Author

mthyman commented Sep 23, 2015

Pull request added #115

@olegagafonov
Copy link

Hi!
Branch 'netty4' peace of code:
// write the pdu out & wait timeout amount of time
ChannelFuture channelFuture = this.channel.writeAndFlush(buffer);
if (configuration.getWriteTimeout() > 0){
channelFuture.await(configuration.getWriteTimeout());
} else {
channelFuture.await();
}
Method writeAndFlush won't throw exception if channel is closed by other side. So, if 'writeTimeout' is default and session listener doesn't handle 'fireChannelClosed' we'll get the same lock.
Please, let me know if @mthyman PR affects branch netty4?

@mthyman
Copy link
Author

mthyman commented Nov 20, 2015

@olegagafonov I haven't tried the netty4 branch myself so I haven't seen the problem there, but looking at your code snippet setting a positive write timeout in your config should avoid the problem if it's there.

One a side note, we've been running a special build with my PR in production for almost two months now without seeing any threads get stuck so far.

@olegagafonov
Copy link

@mthyman You're right! Possitive timeout is a solution. But default timeout is a delayed dead-lock.
Your PR and my code snippet is the same code in differrent branches. But I don't know both branches lifecycle and merge strategy. @jjlauer what do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants