Skip to content
This repository has been archived by the owner on Sep 28, 2018. It is now read-only.

Adding Bugsnag NDK causes some instrumentation tests to crash. #10

Closed
maltzj opened this issue Mar 7, 2018 · 7 comments · Fixed by bugsnag/bugsnag-android#369
Closed

Comments

@maltzj
Copy link

maltzj commented Mar 7, 2018

Expected behavior

Bugsnag ndk doesn't crash when under test

Observed behavior

When I call leaveBreadcrumb with bugsnag ndk support enabled, my app will sometimes crash while testing.

Steps to reproduce

Add

Observable.fromCallable(
                        new Callable<Void>() {
                            @Override
                            public Void call() throws Exception {
                                Bugsnag.leaveBreadcrumb("some non-null value" + "some other non-null value");
                                return null;
                            }
                        })
                .subscribeOn(Schedulers.io())
                .subscribe();

from a Activity under instrumentation test. This doesn't happen all the time, but it's pretty consistently reproducible on this specific test.

Version

NDK 1.1.2 w/ Bugsnag sdk 4.3.1

Additional information

I THINK this is only due to the ndk.

Can't comment on Issues?

Some users have been unable to comment on Github issues when an adblocker extension is enabled.
We recommend temporarily disabling the extension, or if that fails, contacting [email protected].

@fractalwrench
Copy link
Contributor

@maltzj I've created a branch to reproduce this. I've tried running the test both with the leaveBreadcrumb method present, and commented out, and both times I observe:

Test running failed: Instrumentation run failed due to 'Process crashed.'

In RxJava2 there was a change which means it's not permitted to emit null, and attempting to do so will result in a call to onError. Adding a consumer for onError would prevent an uncaught exception from crashing the instrumentation process, e.g.:

.subscribe(System.out::println, Throwable::printStackTrace);

Let me know if I've got the wrong end of the stick here and I can look into this further.

@maltzj
Copy link
Author

maltzj commented Mar 8, 2018

That may be true in RxJava2 land; however, we're currently living in RxJava 1 land (at least for this code). Also, this code doesn't crash in our non-test application, nor does it consistently fail, which is what I'd expect if there were an RxJava bug.

@fractalwrench
Copy link
Contributor

Ah right, my bad! I'll try again with RxJava1 and post results.

@fractalwrench
Copy link
Contributor

The following test reproduces the issue 100% of the time:

public void leaveBreadcrumb() {
        for (int k = 0; k < 100; k++) {
            Observable.fromCallable(
                    new Callable<Void>() {
                        @Override
                        public Void call() throws Exception {
                            Bugsnag.leaveBreadcrumb("some non-null value" + "some other non-null value");
                            return null;
                        }
                    })
                    .subscribeOn(Schedulers.io())
                    .subscribe();
        }
    }

Interestingly using Schedulers.immediate() passes the test, so that may be a potential workaround if you're able to inject the scheduler in for testing purposes. The Logcat seems to suggest that the NDK notifier is trying to free memory which isn't allocated - I'll have to look into this a bit further.

ndk_logcat.txt

@maltzj
Copy link
Author

maltzj commented Mar 19, 2018

gentle poke

@fractalwrench
Copy link
Contributor

@maltzj we've tracked it down to a couple of thread safety issues when adding breadcrumbs, I've linked a PR which partially fixes this if you're interesting in an explanation. Still investigating a more general fix that will prevent this altogether.

@maltzj
Copy link
Author

maltzj commented May 31, 2018

Curious if there are any plans to move forward on this, or is it going to be marked as won'tfix?

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

Successfully merging a pull request may close this issue.

2 participants