mirror of
https://codeberg.org/Mercury-IM/Smack
synced 2024-11-22 14:22:05 +01:00
Do not call synchronized methods in reader/writer thread
This may cause deadlocks with a call to acquire(2) on the introduced readerWriterSemaphore in initConnection(), which is also synchronized.
This commit is contained in:
parent
3d1a781a22
commit
7d2c3ac9f9
2 changed files with 43 additions and 16 deletions
|
@ -297,7 +297,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
private static final AsyncButOrdered<AbstractXMPPConnection> ASYNC_BUT_ORDERED = new AsyncButOrdered<>();
|
protected static final AsyncButOrdered<AbstractXMPPConnection> ASYNC_BUT_ORDERED = new AsyncButOrdered<>();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The used host to establish the connection to
|
* The used host to establish the connection to
|
||||||
|
|
|
@ -921,24 +921,47 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
|
||||||
*
|
*
|
||||||
* @param e the exception that causes the connection close event.
|
* @param e the exception that causes the connection close event.
|
||||||
*/
|
*/
|
||||||
private synchronized void notifyConnectionError(Exception e) {
|
private synchronized void notifyConnectionError(final Exception e) {
|
||||||
|
ASYNC_BUT_ORDERED.performAsyncButOrdered(this, new Runnable() {
|
||||||
|
@Override
|
||||||
|
public void run() {
|
||||||
// Listeners were already notified of the exception, return right here.
|
// Listeners were already notified of the exception, return right here.
|
||||||
if (packetReader.done && packetWriter.done()) return;
|
if (packetReader.done || packetWriter.done()) return;
|
||||||
|
|
||||||
|
// Report the failure outside the synchronized block, so that a thread waiting within a synchronized
|
||||||
|
// function like connect() throws the wrapped exception.
|
||||||
SmackWrappedException smackWrappedException = new SmackWrappedException(e);
|
SmackWrappedException smackWrappedException = new SmackWrappedException(e);
|
||||||
tlsHandled.reportGenericFailure(smackWrappedException);
|
tlsHandled.reportGenericFailure(smackWrappedException);
|
||||||
saslFeatureReceived.reportGenericFailure(smackWrappedException);
|
saslFeatureReceived.reportGenericFailure(smackWrappedException);
|
||||||
maybeCompressFeaturesReceived.reportGenericFailure(smackWrappedException);
|
maybeCompressFeaturesReceived.reportGenericFailure(smackWrappedException);
|
||||||
lastFeaturesReceived.reportGenericFailure(smackWrappedException);
|
lastFeaturesReceived.reportGenericFailure(smackWrappedException);
|
||||||
|
|
||||||
|
synchronized (XMPPTCPConnection.this) {
|
||||||
|
// Within this synchronized block, either *both* reader and writer threads must be terminated, or
|
||||||
|
// none.
|
||||||
|
assert ((packetReader.done && packetWriter.done())
|
||||||
|
|| (!packetReader.done && !packetWriter.done()));
|
||||||
|
|
||||||
// Closes the connection temporary. A reconnection is possible
|
// Closes the connection temporary. A reconnection is possible
|
||||||
// Note that a connection listener of XMPPTCPConnection will drop the SM state in
|
// Note that a connection listener of XMPPTCPConnection will drop the SM state in
|
||||||
// case the Exception is a StreamErrorException.
|
// case the Exception is a StreamErrorException.
|
||||||
instantShutdown();
|
instantShutdown();
|
||||||
|
|
||||||
|
// Wait for reader and writer threads to be terminated.
|
||||||
|
readerWriterSemaphore.acquireUninterruptibly(2);
|
||||||
|
readerWriterSemaphore.release(2);
|
||||||
|
}
|
||||||
|
|
||||||
|
Async.go(new Runnable() {
|
||||||
|
@Override
|
||||||
|
public void run() {
|
||||||
// Notify connection listeners of the error.
|
// Notify connection listeners of the error.
|
||||||
callConnectionClosedOnErrorListener(e);
|
callConnectionClosedOnErrorListener(e);
|
||||||
}
|
}
|
||||||
|
}, XMPPTCPConnection.this + " callConnectionClosedOnErrorListener()");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* For unit testing purposes
|
* For unit testing purposes
|
||||||
|
@ -1248,7 +1271,11 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
|
||||||
LOGGER.info(XMPPTCPConnection.this
|
LOGGER.info(XMPPTCPConnection.this
|
||||||
+ " received closing </stream> element."
|
+ " received closing </stream> element."
|
||||||
+ " Server wants to terminate the connection, calling disconnect()");
|
+ " Server wants to terminate the connection, calling disconnect()");
|
||||||
|
ASYNC_BUT_ORDERED.performAsyncButOrdered(XMPPTCPConnection.this, new Runnable() {
|
||||||
|
@Override
|
||||||
|
public void run() {
|
||||||
disconnect();
|
disconnect();
|
||||||
|
}});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
|
Loading…
Reference in a new issue