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

Windows inproc backend #287

Merged
merged 2 commits into from
Jun 17, 2020

Conversation

eakoli
Copy link
Contributor

@eakoli eakoli commented Jun 10, 2020

Updated inproc backend to support windows via the UnhandledExceptionFilter

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.

This looks a lot simpler than I would have imagined.

Please make sure to enable the integration tests for this:

has_inproc = sys.platform != "win32"

Considering this was the only condition, you might as well just remove all its uses as well.

src/backends/sentry_backend_inproc.c Outdated Show resolved Hide resolved
src/backends/sentry_backend_inproc.c Outdated Show resolved Hide resolved
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.

Also, run this through make format once to fix all the codestyle issues ;-)

tests/conditions.py Outdated Show resolved Hide resolved
@eakoli
Copy link
Contributor Author

eakoli commented Jun 16, 2020

@Swatinem Is there any way for me to get the details about why the checks are failing?

@eakoli eakoli force-pushed the feat-windows-inproc branch from cdbc6d8 to 8d7a4fc Compare June 16, 2020 21:56
@Swatinem
Copy link
Member

https://dev.azure.com/sentry-builds/sentry-native/_build/results?buildId=2458&view=logs&j=7419219a-a3be-5efc-04ce-9ca92267e764&t=ca7f57e0-f409-5f90-1fee-09b0e65be447&l=667

Assertion failed: !bgw->running && bgw->task_count == 0, file D:\a\1\s\src\sentry_sync.c, line 164

And then it was hanging there for 60 minutes. Its the same problem that would be fixed by #284 I think. Thats still waiting for review.

@Mixaill
Copy link
Contributor

Mixaill commented Jun 17, 2020

Yeah, I'm pretty sure that is the same issue which I have with breakpad.

@Swatinem
Copy link
Member

The necessary PR just landed on master. Can you please rebase? Then this should be good to go.

@eakoli eakoli force-pushed the feat-windows-inproc branch from 8d7a4fc to 5b8cc22 Compare June 17, 2020 11:55
@Swatinem Swatinem merged commit b972a24 into getsentry:master Jun 17, 2020
@Swatinem
Copy link
Member

Amazing stuff! Thanks so much ;-)

@eakoli eakoli deleted the feat-windows-inproc branch July 27, 2020 23:33
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