GitHub Issue | n/a |
Type | Bug |
Priority | Critical |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2014-10-03T23:47:49.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 3.4.1, Release 3.4.2, Release 3.5.0, Release 4.0.0 |
Components | Android |
Labels | android, cache, crash, download, googleplay, imageView, module_imageview, qe-manualtest, qe-testadded |
Reporter | Jason Priebe |
Assignee | Ping Wang |
Created | 2014-09-26T21:10:26.000+0000 |
Updated | 2015-07-17T13:02:21.000+0000 |
My app is crashing when I use Google Play Services to deliver doubleclick ads.
[ERROR] : TiDownloadManager: (pool-4-thread-1) [18,3778] Exception downloading http://wwwcache.wral.com/asset/news/local/2014/09/26/14018104/14018104-1411727089-320x180.jpg
[ERROR] : TiDownloadManager: java.lang.ClassCastException: com.android.okhttp.HttpResponseCache cannot be cast to org.appcelerator.titanium.util.TiResponseCache
[ERROR] : TiDownloadManager: at org.appcelerator.titanium.util.TiResponseCache.peek(TiResponseCache.java:187)
[ERROR] : TiDownloadManager: at ti.modules.titanium.ui.widget.TiUIImageView$1.postDownload(TiUIImageView.java:119)
[ERROR] : TiDownloadManager: at org.appcelerator.titanium.util.TiDownloadManager$DownloadJob.run(TiDownloadManager.java:151)
[ERROR] : TiDownloadManager: at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
[ERROR] : TiDownloadManager: at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
[ERROR] : TiDownloadManager: at java.lang.Thread.run(Thread.java:841)
[DEBUG] : dalvikvm: DexOpt: --- END 'ads646200884.jar' (success) ---
(the URL listed in the output is an image that I am loading into an ImageView, so I inferred that the TiDownloadManager is handling the loading of ImageView images)
The problem only goes away if I disable DFP ads (I'm using my own ti.dfp module -
https://github.com/jpriebe/ti.dfp).
I found it strange that it was crashing, because this code was working just a few days ago, and the module had not been changed, nor had I really done anything to the app and the way it handles ImageViews.
I scoured android source code, tweets, and forum posts, and I've learned that in android 4.4, Square's okhttp code is supposedly incorporated into HttpUrlConnection. However, I've never found any code for com.android.okhttp.HttpResponseCache. The only code I can find is for com.squareup.okhttp.HttpResponseCache, not "com.android".
com.squareup.okhttp.HttpResponseCache extends java.net.ResponseCache, just like TiResponseCache does. But the code in TiResponseCache.peek() is only expecting to get back a TiResponseCache object when it calls getDefault(). Changing that might not be trivial, since it depends on the cacheDir member variable of the returned object, and that's not part of the java.net.ResponseCache class.
Here's what I think has happened -- google released Google Play Services 6.1.09 on September 23, 2014. It automatically updated on my device. Inside the new Google Play Services is a call to ResponseCache.setDefault(), and they're setting that default to a com.android.okhttp.HttpResponseCache object.
Titanium assumes that the default system ResponseCache is the one that it set in TiApplication.java, and it doesn't handle things well when somebody else messes with the default.
I don't know what the fix is for this -- I'll leave that to the android platform experts in your employ. But I think it is a critical issue, and will become more obvious in the coming days as this Play Services update propagates. Any app that is using Google Play Services (or at least the DFP portion of it) and the TiDownloadManager will crash.
Jason, I'm chasing a similar dog. Ti.Admob and Ti.Cloudpush, both with updated play library. As soon as I eat, I'll be running tests against the modules, swapping out play libraries to be sure. Crazyness. Just letting you know, you aren't alone. Until you posted, thought it was just me. :) Android S4 (2 of them) running 4.4.2. Google Play Services version 6.1.09 (1459805-038)
The imageView problem ended up being a red herring.
In my case it is Ti.Admob that is causing the error. Tested the android 2.1.1 version using the google play jar packaged with it, and also with the google play jar packaged with Ti.cloudpush. Both cause the error shown above. If it managed to NOT show the error above, it then will throw the error the Jason has documented. @ingo has a test project that fails for meGlad I'm not out in the wilderness on this one. I'm sorry to hear it's impacting you, too, Stephen, but misery does love company. :-) This is scary -- our app is still in development, but if it had been in the wild, all of our users would have started to experience crashes as soon as their phones updated Google Play Services to 6.1.09. And there would have been little we could do to fix it. I would suspect that any app that uses Play Services could be susceptible. I'm using it for DFP, Stephen's using it for Admob and Cloudpush, what about Maps? Any code in the Play Services subsystem that changes out the default responsecache to an okhttp.HttpResponseCache will cause this problem. TiResponseCache is going to have to be modified so that it either maintains its own default cache, separate from the system default cache, or it can otherwise be resilient against changes in the system default cache. Developers with live apps using Google Play Services will need this fix ASAP. I urge Appcelerator to treat this issue with the highest priority.
Hi Jason, Stephen, this drove me crazy over the last few days. Our GialloZafferano Italian Recipes app has MILLIONS of active users here in Italy and abruptly started crashing since September 17th. We use our custom module for DFP using GoogleAds SDK (latest version with Play Services) and immediately discovered that turning off advertising stops the crashes. As Jason wrote, this is a really breaking issue and I hope Appcelerator will come out with a fix in a matter of days. @ingo, if you read this, I think it might be so widespread that it might be worth publishing a note on the blog to alert developers, so that they at least save headaches when they start getting crash reports and user complaints.
Attached admob-Crash.zip as example project.
+1 for the bug report and the critical status. We have a production app that was having some issues which we couldn't reproduce and crash reports like this have soared over the weekend.
I might have a similar case, where it cannot find network related ti.* classes. I'm currently testing if taking out the
nl.rebellic.hockeyapp
module makes a difference. The only other 2 modules in use areyy.logcatcher
anddk.napp.drawer
which both are not network-related.More crashes related to the recent update of the google play services: https://groups.google.com/forum/#!searchin/google-admob-ads-sdk/ava.lang.NoClassDefFoundError$3A$20android.net.http.HttpResponseCache/google-admob-ads-sdk/-3X2MkTD_iA/i_B5FM1P0GsJ (This one is also about HttpResponseCache) https://groups.google.com/forum/#!topic/google-admob-ads-sdk/SX9yb3F_PNk
Perhaps this will help: https://groups.google.com/d/msg/google-admob-ads-sdk/SX9yb3F_PNk/yu89-rSPLp4J Google will release fixes later this week.
Another scenario to add... No crash, but map graphics end up missing in my app. Errors from the console:
The Google Mobile Ads (a.k.a. AdMob) SDK recently enabled http caching, which must have exposed this crash in Titanium. The Mobile Ads SDK has disabled caching for now, but plans to re-enable it in the next version. It looks like Titanium is assuming that any cache object must be a Titanium one when its downcasted (see https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/util/TiResponseCache.java#L187 for one example). This code should probably check instanceof before casting, and assume that the cache is null otherwise.
Eric -- interesting information about the SDK disabling caching and re-enabling it in the next version. When you say that the SDK has disabled caching, has it been disabled in response to the problems caused by the caching code? Can they disable it without pushing a new version of Google Play Services? What is your source for that information?
It is being disabled for now as a result of the Titanium issue. It does require a new play services push (currently no server-side switch), but a new version is being rolled out this week for this as well as some other crashes that were recently introduced. I work at Google and am on the Developer Relations team for the Google Mobile Ads SDK.
Thanks, Eric. I guess you know what you're talking about! :-) I'm glad to hear that Google is interested in the success of Titanium-based apps. I do agree that a fix needs to be made in the Titanium core so that it doesn't make assumptions about what kind of cache is available. Can you shed any light on why I couldn't find source code for com.android.okhttp.HttpResponseCache ? I understand that is code that is being merged in from Square, but the only source I could find was not under the com.android.okhttp namespace.
Besides breaking AdMob, some Ti apps and Google Maps SDK, it also broke the RedLaser SDK. The rollback can't come fast enough!
I'm not sure where the source itself comes from, but android.net.http.HttpResponseCache seems to use it internally: https://github.com/android/platform_frameworks_base/blob/master/core/java/android/net/http/HttpResponseCache.java#L156
master PR: https://github.com/appcelerator/titanium_mobile/pull/6185 3.4.X PR: https://github.com/appcelerator/titanium_mobile/pull/6186
Please see my comments on the commit -- I think that there might be some serious consequences of this code change.
[~jpriebe] to transmit your concern over to the discussion here, yes, the side effect is that this removes the capacity for caching to occur. However, I don't know if we have an alternative. Is there some alternative interface we can code to and respond correctly depending on the type of cache being used?
Eric - If the Mobile Ads SDK enable http caching next release, are you guys planning to expose it somehow? We won't be able to cache on our side if the returned cache is not exposed.
Ingo: is it possible for TiResponseCache to manage its *own* default cache? In other words, can it override setDefault() and instantiate a single object to serve as the cache for the Titanium app? Then whenever it calls getDefault(), it gets a reference to the cache that it created. IIRC, TiResponseCache has all the logic to manage the cached content on the filesystem, so it can function in parallel to any other caches used by other apps/libraries on the system. The downside is that it would not benefit from files cached by other apps on the device. But I would argue that most of the content that a Titanium app is caching is unique to that app, so cross-app caching is of little benefit.
I'm concerned about this because for the class of apps that we build, caching is HUGELY beneficial. We build news apps. If a user opens the app at 13:00 and sees 20 stories, each with an image, and then opens it again at 14:00, it's possible that 15 of the 20 stories and images are the same. Caching saves a lot of data transfer in such a situation, which is very common in our apps.
No caching for remote ImageViews is absolutely a no-go in my opinion. If that's the implication of the fix then a workaround is a must.
Mark - The PRs submitted isn't fixing the problem at hand, but rather applying best coding practices. I believe Google is fixing it internally and will be releasing an update sometime this week. That is my understanding from Eric's comments.
To clarify: * The current PR just addresses the crash. It is a workaround for that. As a side effect, it disables caching, but no caching is better than crashing. * We do have a larger issue for how to handle caching where we are getting a different object than before. In this case, we need some guidance from Google on what is the best path forward here.
For Google Mobile Ads, the SDK calls [HttpResponseCache.getInstalled()](http://developer.android.com/reference/android/net/http/HttpResponseCache.html#getInstalled()) to get the cache. If it doesn't exist, the SDK uses [HttpResponseCache.install](http://developer.android.com/reference/android/net/http/HttpResponseCache.html#install(java.io.File, long)) to create a new one. And according to the documentation, HttpResponseCache.install sets the created cache as the default cache. The [HttpResponseCache implementation](https://github.com/android/platform_frameworks_base/blob/master/core/java/android/net/http/HttpResponseCache.java#L167-L171) is doing the same as the proposed change, which is to check the instance of the default and see if it's an instance of com.android.okhttp.HttpResponseCache, and otherwise returning null. The way this is implemented doesn't play well with other ResponseCache implementations though, since it seems only one type can be the default and the previous type is simply overwritten. Just a thought - Could the TiResponseCache just keep a static TiResponseCache variable that it refers to instead of getDefault()? The class seems to be implementing the whole caching mechanism itself. Why rely on the ResponseCache's default which can get overridden by other implementations?
You mean like I suggested yesterday afternoon? :-)
Ah yes, you did beat me to the suggestion :)
[~jpriebe], [~ericleich], thanks for your suggestion. But Titanium current downloading/caching mechnism for remote images is using URLConnection which is dependent on the system's default cache. If TiResponseCache is not set to the default, images won't be saved to the directory we set.
[~pwang] so is there an alternative solution? No image caching is better than crashes but will still make a poor user experience for many apps.
I hope you can work something out with Google that will allow you to continue caching in one form or another. I can see that you have some additional functions in TiResponseCache (like peek(), for instance) that would make it difficult to rely on just the methods provided by the generic java.net.ResponseCache. There's nothing in the java.net.ResponseCache documentation that says you can't derive your own class from it and use your class instead. But there are obvious problems if you do that and then a library that you use (e.g., Google Play Services) also does it. Some Google guidance would be extremely helpful here. I can work with the disabling of caching for a point release or two (especially since our biggest app yet is still in development), but caching is a critical part of app performance for us. We've *got* to have caching back soon!
To be clear, Google Play services just uses Android's [HttpResponseCache](http://developer.android.com/reference/android/net/http/HttpResponseCache.html) straight up, not a different subclass of java.net.ResponseCache. Is it feasible for TiResponseCache to have a "has a" relationship with HttpResponseCache instead of an "is a" relationship with ResponseCache?
Eric -- you probably know a ton more about this than I do. But from an outsider's perspective, GPS seems to be setting the default cache to an instance of com.android.okhttp.HttpResponseCache. Maybe that's now the "standard" android HttpResponseCache; I don't know. It's really hard to tell from the documentation. TiResponseCache and com.android.okhttp.HttpResponseCache are both derived from java.net.ResponseCache. As long as TiResponseCache has its own methods like peek(), there's not going to be a good way to have it gracefully handle some other module changing the default cache out from under it. I like your idea of the "has a" relationship, but how can it implement things like peek(), if it just "has a" black-box HttpResponseCache? I don't think there's a way to crack it open to see if something exists in cache, for example, to implement something like peek(). And if you look at the implementation of TiResponseCache, I think my peek() example is just the tip of the iceberg.
PR: https://github.com/appcelerator/titanium_mobile/pull/6192 This PR takes care of all kinds of ResponseCache. As long as the system's default response cache is set, we can retrieve the cached content from it. *Caveat:* 1. The cache directory for TiResponseCache is in SDcard. It won't be removed/cleaned until the app is uninstalled. 2. The cache directory for HttpResponseCache depends. It will be removed/cleaned when the app is forced to stop and launched again. For FR: 1. Run the attached app when the network is on. It should not crash and show two images. 2. Back out of the app using BACK button. Turn off the network. Run it again. It should still show two images. This means HttpResponseCache works. 3. Run the code attached below when the network is on. It should show one image. 4. Back out of the app using BACK button. Turn off the network. Run it again. It should still show one image. This means TiResponseCache works.
Ping -- this is great news! Thanks for all your work on this. Question: are there ramifications of peek() returning true when the default cache is not an instance of TiResponseCache? Will that cause the ImageView to think that the image is already on local storage? I notice that ImageView has a nice behavior of deferring image loading until it is scrolled into view (like if it's at the bottom of a TableView, for instance, it doesn't seem to load until the user scrolls down to the bottom of the TableView and brings the ImageView on screen). This change won't affect that, will it?
[~ericleich] Thank you for your guidance so far. We now have a proposed proper fix, but with the update this morning of Google Play Services, I'm not sure how to test it any more. Is there a previous version of Google Play Services we can install?
The Google Play services library 6.1 (rev 20) should be available from Android's SDK manager any day now. If you compile an app against that version of the library and run it on a device that doesn't have Google Play services installed (e.g. an emulator or a Kindle Fire), it'll expose the bug.
[~jpriebe], if the default cache is not an instance of TiResponseCache but it is not null, the peek() function will return true. If peek() returns true but the cached content cannot be fetched successfully, it will get the image through network. This PR won't affect any ImageView/TableView behavior.
[~pwang] would you please set up a backport?
3_4_X PR: https://github.com/appcelerator/titanium_mobile/pull/6200
3.4.1 PR: https://github.com/appcelerator/titanium_mobile/pull/6277
Approved and merged.
When this issue fix will be available? I am still getting this issue when loading image to an imageview from Facebook Album. This is happening in Android Lollipop devices.
This is the error I am getting now.
This issue was fixed last year. The error you're seeing looks like a different problem. Can you post a test case (perhaps create a different ticket altogether)? Thanks,
Hey, I've noticed a similar issue since Lolipop. Previous version of Android are fine so it's likely to be a change in the Android SDK that Titanium hasn't caught yet.
@Jason - Hi, I'm currently with the 4.0.0.GA, I'll update and let you know how it goes.