Improve handling of closing gates/spaces and connection lost #34
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request aims to fix the following issues in relation to closing gates/spaces or if one loses connection when using
KEEP
connections:SpaceRepository.shutdown()
had been called, theSpaceRepository
would in some cases still try to execute new tasks (listen for messages from a client or try to send a response) which throwRejectedExecutionException
's.NEW BEHAVIOR: Tasks are no longer enqueued after
SpaceRepository.shutdown()
has been called.RemoteSpace
was closed (or connection was lost in some way), all threads that earlier had calledget
,query
,put
, ect. on aRemoteSpace
would become stuck (they could be interrupted), because they never got an answer (noInterruptedException
would be triggered).NEW BEHAVIOR: When a
RemoteSpace
is closed, all waiting threads are now interrupted - they throw anInterruptedException
.get
,query
,put
, ect. on aRemoteSpace
AFTER it is closed would make the calling thread stuck (it could be interrupted).NEW BEHAVIOR: An
InterruptedException
is thrown.RemoteSpace.close()
was called, the calling thread would become stuck, unless the thread responsible for receiving messages (KeepClientGate.inboxHandlingMethod()
) got a message. The reason is thatBufferedReader.readLine()
takes a lock that is also needed forBufferedReader.close()
.NEW BEHAVIOR: The underlying socket is now closed first, which effectively also closes the
BufferReader
, so aRemoteSpace
now can be closed without getting stuck.RemoteSpace
was connected to aSpaceRepository
, then callingSpaceRepository.shutdown()
would make the thread responsible for receiving messages (KeepClientGate.inboxHandlingMethod()
) enter an infinite loop (due to the message wasnull
) - effectively wasting one CPU core on nothing.NEW BEHAVIOR: The thread now closes the gate automatically in this case, since a message being
null
meansEOF
(socket closed).Very likely fixes issue #12.
I added a file
TestCloseKeepConnection.java
with six tests to test some of the things above. Each test runs one thread as server and one thread as client, concurrently (they both create even more threads). The tests work most of the time, but also fails too much to be called stable. One problem is that sometimes, some of the tests fail because there are more threads running after the test is finished than there where when it started. Another problem is that the tests sometimes get stuck (runs for ever), because the threads that are joined never terminate. I did not set a timeout, because that would not help making them stable. The second problem only seems to appear when running the tests via Maven in Eclipse, while the first problem is independent of how they are ran. Therefore, these tests are NOT ran when running all tests, because there might still be some concurrency issue.Changes in:
SpaceRepository.java
covers point 1.KeepClientGate.java
covers point 2, 3, 4 and 5.Rendezvous.java
covers point 2.TestRemoteSpace.java
one test was commented out (it never failed when I ran it).TestjSonMessageSerialization.java
removed debug print.TestCloseKeepConnection.java
new tests that are NOT included when running the tests, since they are unstable.