AppAuth-Android icon indicating copy to clipboard operation
AppAuth-Android copied to clipboard

Activity net.openid.appauthdemo.MainActivity has leaked ServiceConnection

Open dewafer opened this issue 8 years ago • 35 comments

I've encountered a leak problem. I was using a simulator without Chrome installed. On a Chrome installed simulator the leak won't happen.

This leak was occurred when I hit the back button and while the MainActivity turned into background.

And when I dig a little deeper into the problem I found that the mBrowserPackage in the BrowserHandler was resolved to com.android.browser and the return value of CustomTabsClient#bindCustomTabsService(...) is false. So the connection is not retained in the BrowserHandler and when BrowserHandler#unbind() is called, nothing is unbind.

I've tried to retained the connection even when CustomTabsClient#bindCustomTabsService(...) returns false. It seems solve the leak problem, but I'm not sure will there be any side effects.

07-06 10:12:45.442 3929-3929/net.openid.appauthdemo E/ActivityThread: Activity net.openid.appauthdemo.MainActivity has leaked ServiceConnection net.openid.appauth.BrowserHandler$1@2206f76 that was originally bound here
                                                                      android.app.ServiceConnectionLeaked: Activity net.openid.appauthdemo.MainActivity has leaked ServiceConnection net.openid.appauth.BrowserHandler$1@2206f76 that was originally bound here
                                                                          at android.app.LoadedApk$ServiceDispatcher.<init>(LoadedApk.java:1092)
                                                                          at android.app.LoadedApk.getServiceDispatcher(LoadedApk.java:986)
                                                                          at android.app.ContextImpl.bindServiceCommon(ContextImpl.java:1303)
                                                                          at android.app.ContextImpl.bindService(ContextImpl.java:1286)
                                                                          at android.content.ContextWrapper.bindService(ContextWrapper.java:604)
                                                                          at android.support.customtabs.CustomTabsClient.bindCustomTabsService(CustomTabsClient.java:60)
                                                                          at net.openid.appauth.BrowserHandler.bindCustomTabsService(BrowserHandler.java:86)
                                                                          at net.openid.appauth.BrowserHandler.<init>(BrowserHandler.java:61)
                                                                          at net.openid.appauth.AuthorizationService.<init>(AuthorizationService.java:101)
                                                                          at net.openid.appauthdemo.MainActivity.onCreate(MainActivity.java:63)
                                                                          at android.app.Activity.performCreate(Activity.java:6237)
                                                                          at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1107)
                                                                          at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2369)
                                                                          at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2476)
                                                                          at android.app.ActivityThread.-wrap11(ActivityThread.java)
                                                                          at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1344)
                                                                          at android.os.Handler.dispatchMessage(Handler.java:102)
                                                                          at android.os.Looper.loop(Looper.java:148)
                                                                          at android.app.ActivityThread.main(ActivityThread.java:5417)
                                                                          at java.lang.reflect.Method.invoke(Native Method)
                                                                          at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
                                                                          at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

dewafer avatar Jul 06 '16 06:07 dewafer

Same problem observed.

hy9be avatar Aug 09 '16 06:08 hy9be

+1

asinel avatar Aug 13 '16 17:08 asinel

I've the same exact problem with v0.3.0 and with v0.4.0 the trace changed a bit:

11-01 15:11:39.431 22219-22219/com.redinput.youtubestats E/ActivityThread: Activity com.redinput.youtubestats.ChannelsActivity has leaked ServiceConnection net.openid.appauth.CustomTabManager$1@7ba31ca that was originally bound here
                                                                           android.app.ServiceConnectionLeaked: Activity com.redinput.youtubestats.ChannelsActivity has leaked ServiceConnection net.openid.appauth.CustomTabManager$1@7ba31ca that was originally bound here
                                                                               at android.app.LoadedApk$ServiceDispatcher.<init>(LoadedApk.java:1136)
                                                                               at android.app.LoadedApk.getServiceDispatcher(LoadedApk.java:1030)
                                                                               at android.app.ContextImpl.bindServiceCommon(ContextImpl.java:1305)
                                                                               at android.app.ContextImpl.bindService(ContextImpl.java:1288)
                                                                               at android.content.ContextWrapper.bindService(ContextWrapper.java:604)
                                                                               at android.support.customtabs.CustomTabsClient.bindCustomTabsService(CustomTabsClient.java:64)
                                                                               at net.openid.appauth.CustomTabManager.bind(CustomTabManager.java:85)
                                                                               at net.openid.appauth.AuthorizationService.<init>(AuthorizationService.java:107)
                                                                               at net.openid.appauth.AuthorizationService.<init>(AuthorizationService.java:85)
                                                                               at net.openid.appauth.AuthorizationService.<init>(AuthorizationService.java:74)
                                                                               at com.redinput.youtubestats.ChannelsActivity.selectAccount(ChannelsActivity.java:96)
                                                                               at com.redinput.youtubestats.ChannelsActivity_ViewBinding$1.doClick(ChannelsActivity_ViewBinding.java:37)
                                                                               at butterknife.internal.DebouncingOnClickListener.onClick(DebouncingOnClickListener.java:22)
                                                                               at android.view.View.performClick(View.java:5201)
                                                                               at android.view.View$PerformClick.run(View.java:21209)
                                                                               at android.os.Handler.handleCallback(Handler.java:739)
                                                                               at android.os.Handler.dispatchMessage(Handler.java:95)
                                                                               at android.os.Looper.loop(Looper.java:148)
                                                                               at android.app.ActivityThread.main(ActivityThread.java:5525)
                                                                               at java.lang.reflect.Method.invoke(Native Method)
                                                                               at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:730)
                                                                               at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:620)

kix2902 avatar Nov 01 '16 14:11 kix2902

I've resolved the problem by using only ONE AuthorizationService for the app. Do not use anonymous AuthorizationService. Hope it'll help =)

blablanumerodeux avatar Nov 02 '16 14:11 blablanumerodeux

@blablanumerodeux can you please elaborate what you have done to use a single AuthorizationService instance. I'm having the same issue in my app. Thanks

madhuteja avatar Nov 23 '16 10:11 madhuteja

When click on system back button from one activity to go another activity then getting this issue. Please resolve as soon as possible.....

04-13 09:33:39.388 25408-25408/com.googleappauth.rahukuma.googlesigninbrowser E/ActivityThread: Activity com.googleappauth.rahukuma.googlesigninbrowser.Main2Activity has leaked ServiceConnection net.openid.appauth.BrowserHandler$1@40d3e4d0 that was originally bound here android.app.ServiceConnectionLeaked: Activity com.googleappauth.rahukuma.googlesigninbrowser.Main2Activity has leaked ServiceConnection net.openid.appauth.BrowserHandler$1@40d3e4d0 that was originally bound here at android.app.LoadedApk$ServiceDispatcher.(LoadedApk.java:969) at android.app.LoadedApk.getServiceDispatcher(LoadedApk.java:863) at android.app.ContextImpl.bindService(ContextImpl.java:1418) at android.app.ContextImpl.bindService(ContextImpl.java:1407) at android.content.ContextWrapper.bindService(ContextWrapper.java:473) at android.support.customtabs.CustomTabsClient.bindCustomTabsService(CustomTabsClient.java:60) at net.openid.appauth.BrowserHandler.bindCustomTabsService(BrowserHandler.java:86) at net.openid.appauth.BrowserHandler.(BrowserHandler.java:61) at net.openid.appauth.AuthorizationService.(AuthorizationService.java:98) at com.googleappauth.rahukuma.googlesigninbrowser.Main2Activity.handleAuthorizationResponse(Main2Activity.java:63) at com.googleappauth.rahukuma.googlesigninbrowser.Main2Activity.checkIntent(Main2Activity.java:44) at com.googleappauth.rahukuma.googlesigninbrowser.Main2Activity.onStart(Main2Activity.java:35) at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1164) at android.app.Activity.performStart(Activity.java:5114) at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2153) at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2230) at android.app.ActivityThread.access$600(ActivityThread.java:141) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1234) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:5041) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:511) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:793) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:560) at dalvik.system.NativeStart.main(Native Method)

I have seen all issue related to this but not able to solve. Please help..

rahulsingh0089 avatar Apr 13 '17 10:04 rahulsingh0089

Should be fixed by #211.

iainmcgin avatar Apr 19 '17 02:04 iainmcgin

Is this issue really solved? I'm still getting the same error... @rahulsingh0089 @madhuteja @blablanumerodeux @kix2902 @asinel @hy9be @dewafer you dont expierience this anymore?

mateuszwlodarczyk265 avatar May 15 '17 13:05 mateuszwlodarczyk265

Are you using 0.6.0 and still seeing the issue? If so, I'll re-open.

iainmcgin avatar May 15 '17 17:05 iainmcgin

@iainmcgin We are using fork of your latest main branch with some changes related with the Fragment response type.

mateuszwlodarczyk265 avatar May 16 '17 11:05 mateuszwlodarczyk265

I am using 0.6.0 and this issue is reproducible.

ghost avatar May 18 '17 02:05 ghost

i am reproducing this issue while using 0.7.0

H3c4t0nch1r3 avatar Nov 13 '17 09:11 H3c4t0nch1r3

This issue still exists :(, and I am using: compile 'net.openid:appauth:0.7.0' here is my stack trace:

Activity activities.MainActivity has leaked ServiceConnection net.openid.appauth.browser.CustomTabManager$1@3b8f611 that was originally bound here android.app.ServiceConnectionLeaked: Activity activities.MainActivity has leaked ServiceConnection net.openid.appauth.browser.CustomTabManager$1@3b8f611 that was originally bound here at android.app.LoadedApk$ServiceDispatcher.(LoadedApk.java:1399) at android.app.LoadedApk.getServiceDispatcher(LoadedApk.java:1294) at android.app.ContextImpl.bindServiceCommon(ContextImpl.java:1503) at android.app.ContextImpl.bindService(ContextImpl.java:1475) at android.content.ContextWrapper.bindService(ContextWrapper.java:688) at android.support.customtabs.CustomTabsClient.bindCustomTabsService(CustomTabsClient.java:70) at net.openid.appauth.browser.CustomTabManager.bind(CustomTabManager.java:95) at net.openid.appauth.AuthorizationService.(AuthorizationService.java:113) at net.openid.appauth.AuthorizationService.(AuthorizationService.java:91) at net.openid.appauth.AuthorizationService.(AuthorizationService.java:80)

awahnteh avatar Nov 26 '17 10:11 awahnteh

Getting the same issue. @iainmcgin

Copy33 avatar Dec 03 '17 08:12 Copy33

reproduce issue with 0.7.0

finchatticus avatar Dec 15 '17 11:12 finchatticus

i'm facing the same problems with 0.7.0... this issue isn't solved at all.

graves501 avatar Dec 28 '17 13:12 graves501

disposing the AuthorizationService helped for me... (edit: I found out when looking at the source of AuthorizationService, maybe add it to the documentation)

PieterIng avatar Dec 29 '17 13:12 PieterIng

@Quxan , can you provide some source code of how you disposed of the AuthorizationService?

awahnteh avatar Dec 29 '17 16:12 awahnteh

@awahnteh in your case in the MainActivity you probably have something like: authService = new AuthorizationService(this);

you can for example dispose it in onDestroy authService.dispose()

the doc describes the following:

/** * Disposes state that will not normally be handled by garbage collection. This should be * called when the authorization service is no longer required, including when any owning * activity is paused or destroyed (i.e. in {@link android.app.Activity#onStop()}). */

PieterIng avatar Dec 29 '17 16:12 PieterIng

Hi @Quxan , I actually did that, but that actually didn't solve the problem for me. In my case it is a bit further complicated by the fact that my instantiation of authService doesn't happen in an activity where I can cleanly dispose of it in onDestroy. It happens deep in the hierarchy of another component, that is instantiated by another component in my hierarchy.

Yes, I imagine climbing down the hierarchy and building some sort of cascade dispose method could be a solution, but not only is that far more complicated than I would prefer in my use case -- it didn't actually solve the problem for me.

What is worse is that I would have loved to know before hand (#documentation!) that AuthorizationService is something that needs to be explicitly disposed upon termination (or onDestroy) of the activity that contains it; that would have changed my component architecture from the get-go than having to go through a lengthy re-architecture exercise.

Ok setting aside my rant for a moment and focusing on some productive sharing ;) for a moment; what I did notice that seemed to work for me is: to not instantiating the AuthorizationService using my activity (in @Quxan's example above that reference is this) as the context. Instead, what I am doing, is I am using my ApplicationContext ( getApplicationContext()) as the context for my AuthorizationService. This ApplicationContext has a far longer lifespan (which is the entire time the application is open), than for example an Activity, which might be destroyed by the operating system when I switch activities). Because of this nature of how the android os works, it is less likely to leak the ServiceConnection cause it is always attached to the ApplicationContext.

Now the real question for Mr. McGinniss (@iainmcgin), does latching my AuthorizationService to my ApplicationContext instead of an Activity have negative repercussions to my app?

awahnteh avatar Dec 30 '17 13:12 awahnteh

Also, a supplemental side bar question for @iainmcgin, in your architecture would changing the relationship between the context and the AuthorizationService from a Strong Reference to a Weak Reference solve this problem?

This is a snippet from the existing source code of AuthorizationService.java's constructor (pay attention to line 117, in the snippet below it will be line 5):

1:  AuthorizationService(@NonNull Context context,
2:                          @NonNull AppAuthConfiguration clientConfiguration,
3:                          @Nullable BrowserDescriptor browser,
4:                          @NonNull CustomTabManager customTabManager) {
5: //--->  mContext = checkNotNull(context); <--- This line here (Line #107 in AuthorizationService.java source code)
6:         mClientConfiguration = clientConfiguration;
7:         mCustomTabManager = customTabManager;
8:         mBrowser = browser;
9: 
10:         if (browser != null && browser.useCustomTab) {
11:             mCustomTabManager.bind(browser.packageName);
12:         }
13:     }

if you detect that the weakReference is broken, mContext == null, you can then at that point cleanly dispose of the Authorization service (or gracefully replace it with an ApplicationContext). or better yet, completely remove the need for passing in a context of any type during the instantiation of the AuthService and simply internally use the Application's context.

I have perused the internals of the AuhtorizationService class and so far nothing has struck me in a way that convinces me that an actual activity context is needed (I could be wrong though as all I did was a quick perusal).

Would love to hear your thoughts/feedback on this.

Cheers & Happy new year!

awahnteh avatar Dec 30 '17 13:12 awahnteh

The issue is still reproducible on the last release.

In my case i do service.dispose(); And i don't see the leakage message in logs anymore.

private AuthorizationService service;

private void authorize() {
        if (mService != null) {
            mService.dispose();
            mService = null;
        }
        service = new AuthorizationService(this, token);
        Intent intent = service.getAuthorizationRequestIntent(getAuthRequest());
        startActivityForResult(intent, REQUEST_AUTH);
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {

            // ...

            AuthorizationResponse resp = AuthorizationResponse.fromIntent(data);
            AuthorizationException ex = AuthorizationException.fromIntent(data);
            if (resp != null) {

                // ...

                service.performTokenRequest(
                        resp.createTokenExchangeRequest(parametersMap),
                        new AuthorizationService.TokenResponseCallback() {
                            @Override
                            public void onTokenRequestCompleted(TokenResponse resp, AuthorizationException ex) {
                                if (resp != null) {
                                    // handle success                        
                                } else {
                                   // handle error
                                }
                                service.dispose();
                                service = null;
                            }
                        });

                // ...
}  

AlexanderDavydov avatar Mar 19 '18 11:03 AlexanderDavydov

Should be marked as "Critical Bug"

arpitjindal97 avatar Jul 12 '18 00:07 arpitjindal97

Ok, coming back to this: it is clear that the issue for most people is that they do not dispose of their AuthorizationService instances at the appropriate time. Though, I do also understand @awahnteh's point that the need to do this needs to be more prominently documented (it is at least documented in the Javadoc for that type, but it's possibly worthy of putting into the main overview docs).

@arpitjindal97 critical in what sense? Is this crashing your app?

I think the real "fix" for this problem for most apps would be if we made it easier to treat an AuthorizationService as a lifecycle-aware component, so that the effects of the dispose() call could be automated. A WeakReference might also be sufficient as a short term fix, I'll think upon that some more.

iainmcgin avatar Aug 23 '18 17:08 iainmcgin

it is clear that the issue for most people is that they do not dispose of their AuthorizationService instances at the appropriate time.

@iainmcgin, when is the appropriate time to dispose it? I face the same issue and in two devices it crashes the app and in one device it works. So that will help me so much.

In those two devices, it crashes immediately after launching chrome tabs. If I blacklist chrome tabs and use normal chrome browser, it doesn't crash though. Only error I get is that one (hence my assumption that it is the cause).

I just wanted to add information/ask in case its relevant

mtangoo avatar Aug 23 '18 18:08 mtangoo

I usually call dispose during onDestroy or onStop, depending on when the instance was created (i.e. in onCreate, onStart or at some later point asynchronously). Usually, onDestroy is your last opportunity to dispose of the instance, otherwise you will see the leak warnings.

iainmcgin avatar Aug 23 '18 20:08 iainmcgin

Thank you

mtangoo avatar Aug 23 '18 20:08 mtangoo

i cant decide this problem too, when i'm login first time and then press "back pressed btn", i have this message "has leaked serviceConnection...", i tried to call "authorizationService.dispose()" in onDestroy, but nothing is changed(

SWRHARD avatar Sep 14 '18 10:09 SWRHARD

still not working, 0.7.1

still not fixed, will be fixed this? it's important problem, cant understand why this is not fixed yet

SWRHARD avatar May 15 '19 04:05 SWRHARD

It worked for me like this using 0.71 private var authService: AuthorizationService? = null

override fun onStop() {
// this needs to be done before returning, not in onActivityResult as described above
// remember the activity ends and then RedirectUriRegisterActivity takes over
        super.onStop()
        authService?.dispose()
        authService = null // prevents next: java.lang.IllegalStateException: Service has been disposed and rendered inoperable
}
<activity
            android:name="net.openid.appauth.RedirectUriReceiverActivity"
            tools:node="replace">
            <intent-filter>
                <action android:name="android.intent.action.VIEW"/>
                <category android:name="android.intent.category.DEFAULT"/>
                <category android:name="android.intent.category.BROWSABLE"/>
                <data android:scheme="yourScheme"/>
            </intent-filter>
        </activity>

Hope this helps anyone.

juanmendez avatar Jan 31 '20 23:01 juanmendez