Last Comment Bug 756735 - java.lang.NullPointerException: at org.mozilla.gecko.Favicons.cancelFaviconLoad(Favicons.java)
: java.lang.NullPointerException: at org.mozilla.gecko.Favicons.cancelFaviconLo...
Status: RESOLVED FIXED
[native-crash][14.0b2]
: crash
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 14 Branch
: ARM Android
: -- critical (vote)
: Firefox 16
Assigned To: Brian Nicholson (:bnicholson)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-18 23:56 PDT by Scoobidiver (away)
Modified: 2012-07-13 15:05 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
synchronize accesses to mLoadTasks (3.27 KB, patch)
2012-06-07 13:12 PDT, Brian Nicholson (:bnicholson)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Scoobidiver (away) 2012-05-18 23:56:32 PDT
There are 3 crashes in 14.0 Beta, the latest one is: bp-919a8a6a-47e1-4f6c-ab65-53ed52120519.

java.lang.NullPointerException
	at org.mozilla.gecko.Favicons.cancelFaviconLoad(Favicons.java:207)
	at org.mozilla.gecko.GeckoApp.maybeCancelFaviconLoad(GeckoApp.java:600)
	at org.mozilla.gecko.GeckoApp.loadFavicon(GeckoApp.java:607)
	at org.mozilla.gecko.GeckoApp.access$000(GeckoApp.java:98)
	at org.mozilla.gecko.GeckoApp$7.run(GeckoApp.java:734)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:150)
	at android.app.ActivityThread.main(ActivityThread.java:4293)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:849)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:607)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.Favicons.cancelFaviconLoad%28Favicons.java%29
Comment 1 Brian Nicholson (:bnicholson) 2012-06-07 13:12:03 PDT
Created attachment 631095 [details] [diff] [review]
synchronize accesses to mLoadTasks

The favicon task is likely getting removed between
    if (!mLoadTasks.containsKey(taskId))
and
    LoadFaviconTask task = mLoadTasks.get(taskId);
causing a NPE when calling task.cancel().
Comment 2 Brian Nicholson (:bnicholson) 2012-06-07 13:14:22 PDT
On a related note, what is the synchronized block here accomplishing?

        public LoadFaviconTask(String pageUrl, String faviconUrl, OnFaviconLoadedListener listener) {
            synchronized(this) {
                mId = ++mNextFaviconLoadId;
            }
            ...
        }

The "this" is an AsyncTask, so this code seems wrong.
Comment 3 Brian Nicholson (:bnicholson) 2012-06-07 17:19:11 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/c0afa3c27907
Comment 4 Lucas Rocha (:lucasr) 2012-06-08 01:56:58 PDT
(In reply to Brian Nicholson (:bnicholson) from comment #2)
> On a related note, what is the synchronized block here accomplishing?
> 
>         public LoadFaviconTask(String pageUrl, String faviconUrl,
> OnFaviconLoadedListener listener) {
>             synchronized(this) {
>                 mId = ++mNextFaviconLoadId;
>             }
>             ...
>         }
> 
> The "this" is an AsyncTask, so this code seems wrong.

Yeah, looks like a thinko, feel free to fix it.
Comment 5 Graeme McCutcheon [:graememcc] 2012-06-08 04:18:40 PDT
https://hg.mozilla.org/mozilla-central/rev/c0afa3c27907

(Merged by Ed Morley)
Comment 6 Brian Nicholson (:bnicholson) 2012-06-13 18:16:56 PDT
Comment on attachment 631095 [details] [diff] [review]
synchronize accesses to mLoadTasks

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: small chance of crashing
Testing completed (on m-c, etc.): 1 week m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Comment 7 Alex Keybl [:akeybl] 2012-06-15 16:00:20 PDT
Comment on attachment 631095 [details] [diff] [review]
synchronize accesses to mLoadTasks

[Triage Comment]
Low risk crash fix - given where we are in the cycle, approving for Aurora 15.
Comment 8 Brian Nicholson (:bnicholson) 2012-07-13 15:05:03 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/26d6082c7e2c

Note You need to log in before you can comment on or make changes to this bug.