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

Close race condition in removeSocket #9

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

rkday
Copy link
Contributor

@rkday rkday commented Oct 18, 2015

While doing some testing, I (rarely) saw a NullPointerException on line 110 of IOHandler.java. From a quick code read, I think I can see a race condition here: socketCreationMap.get(key) could return non-null for Thread 1 on line 109, Thread 2 could then reach line 111 and call socketCreationMap.remove(key), and it could return null on 110 for Thread 1.

This should fix that issue.

@rkday
Copy link
Contributor Author

rkday commented Oct 19, 2015

Stack trace for the issue, by the way:

java.lang.NullPointerException
    at gov.nist.javax.sip.stack.IOHandler.removeSocket(IOHandler.java:110)
    at gov.nist.javax.sip.stack.TCPMessageChannel.close(TCPMessageChannel.java:197)
    at gov.nist.javax.sip.stack.ConnectionOrientedMessageChannel.close(ConnectionOrientedMessageChannel.java:126)
    at gov.nist.javax.sip.stack.TCPMessageProcessor.stop(TCPMessageProcessor.java:172)
    at gov.nist.javax.sip.stack.SIPTransactionStack.removeMessageProcessor(SIPTransactionStack.java:2398)
    at gov.nist.javax.sip.stack.SIPTransactionStack.stopStack(SIPTransactionStack.java:2086)
    at gov.nist.javax.sip.SipStackImpl.stop(SipStackImpl.java:1711)

@@ -106,9 +106,9 @@ protected Socket getSocket(String key) {

protected void removeSocket(String key) {
socketTable.remove(key);
if ( socketCreationMap.get(key) != null ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinf of preliminar checking use to be done to enhance performance. After initial light checking, it needs to recheck in proper critical section, which its what is missing here, hence NPE

deruelle added a commit that referenced this pull request Nov 3, 2015
Close race condition in removeSocket
@deruelle deruelle merged commit 17e8284 into RestComm:master Nov 3, 2015
deruelle referenced this pull request in mobius-software-ltd/corsac-sip Jan 19, 2024
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

Successfully merging this pull request may close these issues.

3 participants