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

=rem #17729 Don't use Implicits.global in remoting shutdown #17876

Merged
merged 1 commit into from
Jul 3, 2015

Conversation

patriknw
Copy link
Member

@patriknw patriknw commented Jul 1, 2015

I have verified (with printlns) that the callback is running with the system.dispatcher in all cases when running the tests in akka-remote.

@@ -146,6 +145,7 @@ private[remote] class Remoting(_system: ExtendedActorSystem, _provider: RemoteAc
endpointManager = None
}

import system.dispatcher
Copy link
Member

Choose a reason for hiding this comment

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

is it still alive and kicking here?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see, it is. endpointManager is an actor.
I have verified (with printlns) that the callback is running with the system.dispatcher in all cases when running the tests in akka-remote.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think the caller of this shutdown() method actually blocks on an Await, so this is probably safe.

@akka-ci akka-ci added the validating PR is currently being validated by Jenkins label Jul 1, 2015
@patriknw
Copy link
Member Author

patriknw commented Jul 1, 2015

Refs #17729

@akka-ci akka-ci added the tested PR that was successfully built and tested by Jenkins label Jul 1, 2015
@akka-ci
Copy link

akka-ci commented Jul 1, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/3086/

@akka-ci akka-ci removed the validating PR is currently being validated by Jenkins label Jul 1, 2015
@drewhk
Copy link
Member

drewhk commented Jul 3, 2015

LGTM

@ktoso
Copy link
Member

ktoso commented Jul 3, 2015

LGTM, nice detective work!

ktoso added a commit that referenced this pull request Jul 3, 2015
=rem #17729 Don't use Implicits.global in remoting shutdown
@ktoso ktoso merged commit 49e6e7f into master Jul 3, 2015
@ktoso ktoso deleted the wip-17729-Implicits.global-patriknw branch July 3, 2015 09:47
ktoso added a commit to ktoso/akka that referenced this pull request Jan 11, 2016
…triknw

=rem akka#17729 Don't use Implicits.global in remoting shutdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants