Skip to content

fix: Native layer not closing #1092

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

Merged
merged 10 commits into from
Dec 5, 2022
Merged

fix: Native layer not closing #1092

merged 10 commits into from
Dec 5, 2022

Conversation

bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented Nov 24, 2022

On mobile, the native layer is getting set up during buildtime and is self-initializing.
If the SDK gets disabled during runtime (i.e. in the Configure callback, then the native layer needs to be closed down too.

Additionally, there was no way to close the SDK and have the native layer close with it.

@bitsandfoxes bitsandfoxes requested a review from vaind December 5, 2022 10:55
Copy link
Collaborator

@vaind vaind left a comment

Choose a reason for hiding this comment

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

It's OK, though maybe we could clean SentryUnity class a bit by extracting a non-static class with all the current code and have a single variable private static SentryUnitySdk? Instance that we set in init() and remove in close() (+ call its close() of course).
SentryUnitySdk could have non-static and non-nullable Options, SentrySdk and FileStream lockfile`
WDYT?

@bitsandfoxes bitsandfoxes mentioned this pull request Dec 5, 2022
@bitsandfoxes
Copy link
Contributor Author

It's OK, though maybe we could clean SentryUnity class a bit by extracting a non-static class with all the current code and have a single variable private static SentryUnitySdk? Instance that we set in init() and remove in close() (+ call its close() of course). SentryUnitySdk could have non-static and non-nullable Options, SentrySdk and FileStream lockfile` WDYT?

Excellent point.

@bitsandfoxes bitsandfoxes merged commit d0016c2 into main Dec 5, 2022
@bitsandfoxes bitsandfoxes deleted the fix/close-mobile-native branch December 5, 2022 13:35
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