Closed infeo closed 3 years ago
I implemented some of the suggestions. Please try if you can reproduce the issue using the latest changes.
Hey @hypfvieh!
First of all a huge thank you for providing this lib and maintaining it with such quick replies!
I had a chance to test your changes, but unfortunately that didn't play out.
I documented my test here: https://github.com/cryptomator/integrations-linux/issues/2
Are you sure you use the latest git revision?
The line numbers reported in the exception do not match.
e.g.AbstractConnection.java:946
is an empty line,
AbstractConnection.java:920
is the closing bracket of a catch block ...
AbstractConnection.java:709
is a piece of javadoc
IncomingMessageThread.java:43
this is the only correct line, but the the only change in that class was to add a volatile
to the boolean member, so line numbers are likely the same as before
Your were absolutely right @hypfvieh. I started a wrong cryptomator build with old dependencies.
I was quite confused yesterday evening as I couldn't observe the RejectedExecutionException
in my secret-service
tests anymore, but still with the wrong cryptomator build. But in my secret-serivice tests the RejectedExecutionException
occurs less frequently, as when integrated with cryptomator.
So your changes actually have fixed the
Maybe it would be worth to add a regression test for that @hypfvieh?RejectedExecutionException
issue.
But I still have problems on tearing down, as documented here.
I still can trigger the RejectedExecutionException
and the blocked shutdown:
https://github.com/cryptomator/integrations-linux/issues/2#issuecomment-726844687
Seems to be fixed, see cryptomator/integrations-linux#2
Summary
There is a possible race condition when closing the connection to the DBus via the
DBusConnection
class leading to anjava.util.concurrent.RejectedExecutionException
.Description
In https://github.com/swiesend/secret-service/issues/21 a problem is encountered where after a successfully established DBus connection with the
DBusConnection
class and the (later happend) disconnect, still at least one message is tried to handled. Because the appearance of the problem is intermittent, it may be a race condition. The exception stacktrace:It seems that the
workerThreadPoolExecutor
was already terminated when the message should be handled. Looking at docs of the ThreadPoolExecutor, there are three cases where it is shutdown:shutdown()
shutdownNow()
The first two cases happen only in either in the
changeThreadCount()
ordisconnectInternal()
method of theAbstractConnection
class. The first one basically transfers all jobs to a new executor in a thread safe manner, hence it should impose no problem.The second,
disconnectInternal()
, is therefore the one I looked at. The DBusConnection is an object which recieves, send and handle messages. Currently, first the handling is closed and afterwards the recieving part. https://github.com/hypfvieh/dbus-java/blob/ecb3ad04893f52908ffae1c7251e68357f1e1d3d/dbus-java/src/main/java/org/freedesktop/dbus/connections/AbstractConnection.java#L542-L563The message source is the
IncomingMessageThread
. This object is terminated via a variable:While I'm not a fan of closing the message source after closing the handling, more critical is that neither the
connected
variable norterminate
are volatile. This could lead to a cache coherence problem (see https://www.baeldung.com/java-volatile#volatile).Suggested Fix
Make the variables
connected
andterminate
volatile.Additionally, i suggest to first close the message source (setting
terminate
tofalse
) before shutting down the executor.As a third point, maybe the
setTerminate(boolean _terminate)
method can be refactored toterminate()
to only terminate the connection and not setting it to any value.