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

FirstChanceHandler execute once #95

Closed
wants to merge 1 commit into from

Conversation

stima
Copy link

@stima stima commented Feb 9, 2024

For platfroms that support FirstChanceHandler (windows, linux) execute it only once.

For windows that mimics behaviour of sentry__crashpad_handler but avoid an issue with a possible race condition for multiple executions of sentry__crashpad_handler:

  • event creation on for a path crashpad_state_t::event_path during crashpad_backend_flush_scope
  • crashpad_state_t::event inside sentry__crashpad_handler

That would also remove necessety of having sentry__enter_signal_handler for linux, because crashpad has same logic inside SignalHandler::HandleOrReraiseSignal.

@stima stima requested a review from a team February 9, 2024 16:10
@supervacuus
Copy link
Collaborator

Thanks, @stima and @ayx-rsavchenko! The way we currently set up our handlers, this makes sense. I have a couple of other changes in the same territory and will look at how to integrate this best when I am working on those.

@stima
Copy link
Author

stima commented Jun 6, 2024

@supervacuus Hey any updates on that?

@supervacuus
Copy link
Collaborator

@supervacuus Hey any updates on that?

Hi @stima, I am truly sorry for not responding for such a long time. Over the last months, the focus has been on supporting downstream SDKs with various native topics that weren't necessarily Native SDK-related. This led to open PRs and issues related to standalone usage being put on the back burner. I will focus on standalone usage this month.

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

lgtm

@supervacuus
Copy link
Collaborator

Closing in favor of #109 and getsentry/sentry-native#1019 which integrated the general idea of the PR. Thanks!

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

Successfully merging this pull request may close these issues.

4 participants