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

fix endless loop on crash after init during +load #338

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cynicer
Copy link

@cynicer cynicer commented Jan 16, 2025

Things to consider before you submit the PR:

  • Has CHANGELOG.md been updated?
  • Are tests passing locally?
  • Are the files formatted correctly?
  • Did you add unit tests?
  • Did you test your change with the sample apps?

Description

This PR fixes an issue in the PLCrashReporter, where a static initialization of units (in the +load method), could lead to an endless loop when trying to handle a crash.
This happens because the crash callback handlers are set even though the static shared_handler_context is not initialized yet. Once the shared_handler_context gets initialized, the list of callbacks initialized as empty. This leads to an endless loop, as no callback to handle a crash is available.
The underlying issue is connected to the (Static Initialization Order Fiasco)[https://en.cppreference.com/w/cpp/language/siof], due to initialization of static objects from different translation units when one depends on the other being initialized already.

Solution

We need to force the static struct (shared_handler_context) to be initialized on first access. This PR achieves this by adapting the shared_handler_context to be used as a function and adapts its usage throughout the PLCrashSignalHandler. Using this approach, we can centralize the initialization and enforce a proper order, resolving the underlaying issue.

Related PRs or issues

#325

Misc

All tests are running through, except testStackWalkerRegression, but this one also fails for me locally on master.
Not sure how to properly unit test this since it's pretty hard to reproduce and needs another app to load it during it's load.

@cynicer cynicer requested a review from a team as a code owner January 16, 2025 15:21
@cynicer cynicer changed the title refactor init of shared_handler_context to force correct order during early init fix endless loop on crash after init during +load Jan 16, 2025
@cynicer
Copy link
Author

cynicer commented Jan 16, 2025

@microsoft-github-policy-service agree company="Dynatrace"

@IlyaBausovAkvelon
Copy link
Contributor

Hi @cynicer,
Thank you for your contribution!
I've tested it and now it does not face this endless loop that was present when we were debugging you original issue. Also I've checked that everything else is working.

@cynicer
Copy link
Author

cynicer commented Jan 27, 2025

@IlyaBausovAkvelon Thanks for the effort. It seems like another approving review is needed. Who should I/we best tag to do that?

Also, is there a timeline for release? The last release was a while ago, and I'd appreciate this bugfix being released soon. It kind of blocks me.

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.

2 participants