Skip to content

Windows inproc backend#287

Merged
Swatinem merged 2 commits intogetsentry:masterfrom
NetsoftHoldings:feat-windows-inproc
Jun 17, 2020
Merged

Windows inproc backend#287
Swatinem merged 2 commits intogetsentry:masterfrom
NetsoftHoldings:feat-windows-inproc

Conversation

@eakoli
Copy link
Copy Markdown
Contributor

@eakoli eakoli commented Jun 10, 2020

Updated inproc backend to support windows via the UnhandledExceptionFilter

Swatinem
Swatinem reviewed Jun 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@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:

sentry-native/tests/conditions.py

Line 10 in b85641b

has_inproc = sys.platform != "win32"

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

Swatinem
Swatinem reviewed Jun 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@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 ;-)

@eakoli
Copy link
Copy Markdown
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
Copy Markdown
Contributor

Swatinem commented Jun 17, 2020

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
Copy Markdown
Contributor

Mixaill commented Jun 17, 2020

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

@Swatinem
Copy link
Copy Markdown
Contributor

Swatinem commented Jun 17, 2020

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
Copy Markdown
Contributor

Swatinem commented Jun 17, 2020

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

Reviewers

1 more reviewer

@Swatinem Swatinem Swatinem approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eakoli @Swatinem @Mixaill