Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP thread conccurency/synchronization issue #77

Open
lmcdasi opened this issue Jan 17, 2022 · 5 comments
Open

TCP thread conccurency/synchronization issue #77

lmcdasi opened this issue Jan 17, 2022 · 5 comments

Comments

@lmcdasi
Copy link

lmcdasi commented Jan 17, 2022

I have threading issue when using TCP. The issue arrives when jain sip stack sends the 1st SIP out via SipDialog.sendRequest:

(a) thread#1 executes: sendTCPMessage which calls sendBytes (clientSock = null -> openOutgoingConnection) -> writeChunks -> send
In "NioTcpMessageProcessor.send" at then end it will execute: selector.wakeup
(b) thread#2 wakes up since it is on selector.select and will execute write. In the write method line #214 nioHandler.getMessageChannel will return null and thus have a 'Dead socketChannel'
(c) thread#1 returns in sendTCPMessage and since sock != socketChannel 1st time it will excute:
nioHandler.putMessageChannel(sock, this);

But then this is too late. The 1st SipDialog.sendRequest is not sent out & the stack does not even retry because it does not detect this as a failure.
I have added myself a stack trace in the putMessageChannel to confirm that this is what happens.

siptcperror.log

@lmcdasi
Copy link
Author

lmcdasi commented Jan 19, 2022

NIOHandler - openOutgoingConnection method, connected boolean is wrong. On line #379 replacing connected in if with:

if (attempted && (clientSock!= null && clientSock.isBlocking() && !clientSock.isConnected()))

would be more appropriate. The removeSocket in finally should happen only if clientSock is blocking & it is not connected.

@vladimirralev
Copy link
Collaborator

Hey, thanks. Looks good to me. Please do a pull request I will try to get to it in the next week or so.

@lmcdasi
Copy link
Author

lmcdasi commented Jan 27, 2022 via email

@lmcdasi
Copy link
Author

lmcdasi commented Feb 10, 2022

Hi, any update regarding the merge request ? I really need this fix. Thx.

@YiuTerran
Copy link

@vladimirralev please review the pr...

kpouer pushed a commit to kpouer/jsip that referenced this issue Jan 8, 2023
(cherry picked from commit 1d61634afe6620e1a8367c063ae40c6772091d84)
kpouer pushed a commit to kpouer/jsip that referenced this issue Jan 15, 2023
(cherry picked from commit 1d61634afe6620e1a8367c063ae40c6772091d84)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants