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

Automatic framework shutdown may leak bundle threads #264

Open
saschazelzer opened this issue Mar 23, 2018 · 1 comment
Open

Automatic framework shutdown may leak bundle threads #264

saschazelzer opened this issue Mar 23, 2018 · 1 comment

Comments

@saschazelzer
Copy link
Member

In the current implementation, the framework is automatically shut down when the last Bundle instance goes out of scope (if it wasn't shut down explicitly before). The cleanup logic also ensures that the last Bundle instance going out of scope will wait for all shut-down operations to complete.

The above was deliberately designed to be thread-safe, such that Bundle (and Framework) instances may be freely copied and stored in different threads with the shut-down logic being thread-safe.

However, when used in a naive (and typical) way, this may lead to threads being leaked on process termination and depending on the platform hence may lead to abnormal termination. This may happen when the main thread exits while a thread created by the framework is still active. This is easily possible if the thread created by the framework (a "bundle thread") still keeps a Bundle instance around and hence does prevent the main thread from shutting down the framework itself (and waiting for it to complete). Instead, the shut-down is triggered by the other thread (if the process has not terminated yet) but races with the main thread which may be exiting and terminating the process.

While that particular problem can be avoided by keeping a Framework instance around in the main thread and call Shutdown and WaitForStop before exiting main, I propose the following change:

  1. Make the Framework a move-only type
  2. Manage the implicit framework shut-down and wait logic in the Framework destructor only
  3. Document best practices how to create Framework instances and how to manage their life-time

This I believe should make the typical use case less error prone.

Note that framework objects like Bundle, BundleContext, BundleEvent, etc. are already meant to be safe-guarded when being used after the framework was shut down. However, this should be double-checked when working on this issue.

@jeffdiclemente
Copy link
Member

@saschazelzer Thanks for logging this issue in detail.

I'd like to first understand at which point during process termination we are talking about in this case. Is this before std::exit() is called? Is it during std::exit()? At some other point? all of the above?
Is there a reproducible test case I could look at?

I can see how the proposed changes would solve the problem if the Framework object was never moved from the main thread. I still see a problem when the main thread enters std::exit() and the Framework object is owned by any other thread. Is this solution intended to not solve this problem?

"However, when used in a naive (and typical) way, this may lead to threads being leaked on process
termination and depending on the platform hence may lead to abnormal termination."

Isn't abnormal termination only possible if the Framework hasn't been destroyed prior to the process
entering std::exit()? Is abnormal termination possible before std::exit()? If so, could you
provide an example?

"2. Manage the implicit framework shut-down and wait logic in the Framework destructor only"

Does this imply that the CoreBundleContextHolder code in FrameworkFactory is removed?

  1. Document best practices how to create Framework instances and how to manage their life-time

What would be the best practices?

If racing with bundle threads are part of the problem, is there anyway to limit the amount of threads which are spawned during Framework shutdown?

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

No branches or pull requests

2 participants