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

[PLAT-8191] Add Bugsnag.isStarted #1640

Merged
merged 3 commits into from
Mar 30, 2022
Merged

Conversation

tomlongridge
Copy link
Contributor

Goal

From #1621:

Add Bugsnag.isStarted so that it is possible to check whether it is safe to call the other methods. Currently to use Bugsnag safely you either have to keep track of this yourself or wrap calls in try/catch.

Raised internally for CI and merge purposes.

@tomlongridge tomlongridge requested a review from lemnik March 29, 2022 12:32
@tomlongridge tomlongridge changed the title Jallen/bugsnag isstarted Add Bugsnag.isStarted Mar 29, 2022
@tomlongridge tomlongridge changed the title Add Bugsnag.isStarted [PLAT-8191] Add Bugsnag.isStarted Mar 29, 2022
@tomlongridge tomlongridge force-pushed the jallen/bugsnag-isstarted branch from 33d9ee7 to a427799 Compare March 29, 2022 15:57
Copy link
Contributor

@lemnik lemnik left a comment

Choose a reason for hiding this comment

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

LGTM

@tomlongridge tomlongridge merged commit f74d35a into next Mar 30, 2022
@tomlongridge tomlongridge deleted the jallen/bugsnag-isstarted branch March 30, 2022 10:08
@lemnik lemnik mentioned this pull request Mar 31, 2022
* other methods will throw IllegalStateException.
*/
public static boolean isStarted() {
return client != null;
Copy link

@jgreen210 jgreen210 Jan 17, 2023

Choose a reason for hiding this comment

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

This isn't thread safe - it's using the field directly without obtaining the lock. It could be made thread safe by using getClient() not client. I made #1795 for this.

Choose a reason for hiding this comment

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

You could use a https://developer.android.com/reference/androidx/annotation/GuardedBy annotation on the client field to document the locking requirement.

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.

4 participants