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

Improved fix for iPOPO issue 100 #101

Merged
merged 2 commits into from
Sep 4, 2018
Merged

Improved fix for iPOPO issue 100 #101

merged 2 commits into from
Sep 4, 2018

Conversation

scottslewis
Copy link
Collaborator

This provides an improved/general fix for issue 100: #100

What was happening was that the thread used to read remote service discovery messages from Java (in the py4j provider in rsa.providers.distribution.py4j.py) was not being cleanly shutdown upon framework shutdown (i.e. distribution provider invalidate). With this addition it is now being shut down cleanly and I am unable to reproduce the shutdown hang.

@scottslewis
Copy link
Collaborator Author

Hi Thomas. I investigated this issue further. It seems that two things were wrong: 1) the call to executor.shutdown() was being made in the incorrect location (in unimport rather than container invalidate) and ; 2) The distribution provider daemon thread created to read Java-based remote service discovery messages was not being shutdown properly, and with some sequencing would remain running (I'm not sure why it would remain running given that it was a daemon thread, but Python threading is somewhat different from what I'm used to). I did reproduce the shutdown hang, and it was reliably hanging because this thread wasn't being stopped/exiting correctly. With this pull request it is, and I can no longer reproduce the exit hang.

After all, it did not seem to be a problem with osgiservicebridge code, but rather it was in the py4j distribution provider. So I'm going to close this issue once through the pull request: ECF/Py4j-RemoteServicesProvider#2 Thanks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 87.136% when pulling 7c94db0 on rsa-integration into b83e801 on master.

@tcalmant
Copy link
Owner

tcalmant commented Sep 4, 2018

Hi Scott,
That's great 👍
Sorry about the issue in osgiservicebridge, I thought it was a possible blocking case in the use of thread conditions.

I'll merge this fix & bump code to 0.8.1.
I'll wait just a bit before releasing it, just in case I get some news on #99.

@tcalmant tcalmant merged commit 28bafd7 into master Sep 4, 2018
@tcalmant tcalmant added this to the 0.8.1 milestone Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants