AppAuth-Android
AppAuth-Android copied to clipboard
Activity net.openid.appauthdemo.MainActivity has leaked ServiceConnection
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)
Same problem observed.
+1
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)
I've resolved the problem by using only ONE AuthorizationService for the app. Do not use anonymous AuthorizationService. Hope it'll help =)
@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
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.
I have seen all issue related to this but not able to solve. Please help..
Should be fixed by #211.
Is this issue really solved? I'm still getting the same error... @rahulsingh0089 @madhuteja @blablanumerodeux @kix2902 @asinel @hy9be @dewafer you dont expierience this anymore?
Are you using 0.6.0 and still seeing the issue? If so, I'll re-open.
@iainmcgin We are using fork of your latest main branch with some changes related with the Fragment response type.
I am using 0.6.0 and this issue is reproducible.
i am reproducing this issue while using 0.7.0
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.
Getting the same issue. @iainmcgin
reproduce issue with 0.7.0
i'm facing the same problems with 0.7.0... this issue isn't solved at all.
disposing the AuthorizationService helped for me... (edit: I found out when looking at the source of AuthorizationService, maybe add it to the documentation)
@Quxan , can you provide some source code of how you disposed of the AuthorizationService?
@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()}). */
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?
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!
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;
}
});
// ...
}
Should be marked as "Critical Bug"
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.
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
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.
Thank you
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(
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
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.