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

Fix cm thread exit #404

Merged
merged 3 commits into from Jun 28, 2018

Conversation

2 participants
@paddybyers
Copy link
Member

paddybyers commented Jun 28, 2018

Fix the synchronisation around destruction of the connection manager thread and subsequent recreation.

@paddybyers paddybyers requested a review from funkyboy Jun 28, 2018

while(!state.terminal && stateChange == null) {
/* wait for a state change event or for expiry of the current state */
tryWait(state.timeout);
while(!exiting && stateChange == null) {

This comment has been minimized.

@funkyboy

funkyboy Jun 28, 2018

Contributor

Aren't we already in a !exiting context given https://github.com/ably/ably-java/pull/404/files?utf8=%E2%9C%93&diff=split#diff-060685a947a7032665de7ac9d33bb6ffR838 ? Do we need this double check?

This comment has been minimized.

@paddybyers

paddybyers Jun 28, 2018

Member

All we know is that we were !exiting when we entered the loop at the top. Since then we will have waited for state changes, so it's no longer expected to be true.

@funkyboy
Copy link
Contributor

funkyboy left a comment

It doesn't introduce any new failing tests. Failures in this build are already known and being addressed in https://github.com/ably/ably-java/tree/fix-some-flaky-tests-ci

@funkyboy

This comment has been minimized.

Copy link
Contributor

funkyboy commented Jun 28, 2018

Merging now so I can rebase and see if the reauth_fail() test is now passing.

@funkyboy funkyboy merged commit 59100ab into develop Jun 28, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@funkyboy funkyboy deleted the fix-cm-thread-exit branch Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment