search

tcalmant / ipopo

iPOPO: a Service-Oriented Component Model for Python
https://ipopo.readthedocs.io/
Apache License 2.0
69 stars 28 forks source link

Improved fix for iPOPO issue 100 #101

Closed scottslewis closed 6 years ago

scottslewis commented 6 years ago

This provides an improved/general fix for issue 100: https://github.com/tcalmant/ipopo/issues/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 commented 6 years ago

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: https://github.com/ECF/Py4j-RemoteServicesProvider/issues/2 Thanks.

coveralls commented 6 years ago

Coverage Status

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

tcalmant commented 6 years ago

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.