Closed Bug 1354968 Opened 7 years ago Closed 7 years ago

Intermittent test_quit_restart.py TestQuitRestart.test_in_app_restart_with_callback | application crashed [@ PLDHashTable::Add(void const*, mozilla::fallible_t const&)]

Categories

(Toolkit :: Safe Browsing, defect, P1)

Product:
Toolkit
Component:
Safe Browsing
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: hchang)

References

Details

(Keywords: crash, intermittent-failure)

Attachments

(1 file)

Bug 1354968 - Avoid concurrent access of mTableRefreshness.
59 bytes, text/x-review-board-request
francois
: review+
Details 
First frames of the crashing thread:

17:01:15     INFO - Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
17:01:15     INFO - Crash address: 0x0
17:01:15     INFO - Process uptime: 7 seconds
17:01:15     INFO - 
17:01:15     INFO - Thread 35 (crashed)
17:01:15     INFO -  0  XUL!PLDHashTable::Add(void const*, mozilla::fallible_t const&) [PLDHashTable.h:a1e5043844f2 : 132 + 0x0]
17:01:15     INFO -     rax = 0x0000000000000000   rdx = 0x00007fff717561f8
17:01:15     INFO -     rcx = 0x0000000000000000   rbx = 0x0000015b55299aca
17:01:15     INFO -     rsi = 0x0000370000003700   rdi = 0x0000360000003703
17:01:15     INFO -     rbp = 0x000000011f980c10   rsp = 0x000000011f980be0
17:01:15     INFO -      r8 = 0x000000011f980b90    r9 = 0x000000011f981000
17:01:15     INFO -     r10 = 0x00007fff821013ef   r11 = 0x00007fff821013c0
17:01:15     INFO -     r12 = 0x000000011f980d18   r13 = 0x000000011f2644c8
17:01:15     INFO -     r14 = 0x000000011f2b9090   r15 = 0x000000011f2b9090
17:01:15     INFO -     rip = 0x00000001009eb127
17:01:15     INFO -     Found by: given as instruction pointer in context
17:01:15     INFO -  1  XUL!mozilla::safebrowsing::Classifier::SetLastUpdateTime(nsACString const&, unsigned long long) [nsTHashtable.h:a1e5043844f2 : 161 + 0xf]
17:01:15     INFO -     rbx = 0x0000015b55299aca   rbp = 0x000000011f980c60
17:01:15     INFO -     rsp = 0x000000011f980c20   r12 = 0x000000011f980d18
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x000000011f264480
17:01:15     INFO -     r15 = 0x000000011f2b9090   rip = 0x0000000104a2f09a
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  2  XUL!nsUrlClassifierDBServiceWorker::SetLastUpdateTime(nsACString const&, unsigned long long) [nsUrlClassifierDBService.cpp:a1e5043844f2 : 956 + 0xb]
17:01:15     INFO -     rbx = 0x000000011ec926c0   rbp = 0x000000011f980c90
17:01:15     INFO -     rsp = 0x000000011f980c70   r12 = 0x000000011f980d18
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x0000015b55299aca
17:01:15     INFO -     r15 = 0x000000011f2b9090   rip = 0x0000000104a3df20
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  3  XUL!UrlClassifierDBServiceWorkerProxy::SetLastUpdateTimeRunnable::Run() [nsUrlClassifierProxies.cpp:a1e5043844f2 : 240 + 0x8]
17:01:15     INFO -     rbx = 0x0000000000000000   rbp = 0x000000011f980ca0
17:01:15     INFO -     rsp = 0x000000011f980ca0   r12 = 0x000000011f980d18
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x000000011bf0f110
17:01:15     INFO -     r15 = 0x000000011bf0f080   rip = 0x0000000104a46d45
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  4  XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:a1e5043844f2 : 1269 + 0x6]
17:01:15     INFO -     rbx = 0x0000000000000000   rbp = 0x000000011f980da0
17:01:15     INFO -     rsp = 0x000000011f980cb0   r12 = 0x000000011f980d18
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x000000011bf0f110
17:01:15     INFO -     r15 = 0x000000011bf0f080   rip = 0x0000000100a6b05c
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  5  XUL!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp:a1e5043844f2 : 389 + 0xd]
17:01:15     INFO -     rbx = 0x0000000000000001   rbp = 0x000000011f980dc0
17:01:15     INFO -     rsp = 0x000000011f980db0   r12 = 0x000000011bf0f080
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x000000011f8042c0
17:01:15     INFO -     r15 = 0x0000000000000000   rip = 0x0000000100a6960f
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  6  XUL!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [MessagePump.cpp:a1e5043844f2 : 368 + 0xd]
17:01:15     INFO -     rbx = 0x000000011ef6de80   rbp = 0x000000011f980e20
17:01:15     INFO -     rsp = 0x000000011f980dd0   r12 = 0x000000011bf0f080
17:01:15     INFO -     r13 = 0x000000011f8042e0   r14 = 0x000000011f8042c0
17:01:15     INFO -     r15 = 0x0000000000000000   rip = 0x000000010100f272
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  7  XUL!MessageLoop::Run() [message_loop.cc:a1e5043844f2 : 231 + 0x5]
17:01:15     INFO -     rbx = 0x000000011bf0f080   rbp = 0x000000011f980e50
17:01:15     INFO -     rsp = 0x000000011f980e30   r12 = 0x0000000000000000
17:01:15     INFO -     r13 = 0x00000000000008ff   r14 = 0x000000011bf0f0a0
17:01:15     INFO -     r15 = 0x000000011ef6de80   rip = 0x0000000100fc1e5c
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  8  XUL!nsThread::ThreadFunc(void*) [nsThread.cpp:a1e5043844f2 : 500 + 0x5]
17:01:15     INFO -     rbx = 0x000000011bf0f080   rbp = 0x000000011f980ec0
17:01:15     INFO -     rsp = 0x000000011f980e60   r12 = 0x0000000000000000
17:01:15     INFO -     r13 = 0x00000000000008ff   r14 = 0x000000011bf0f0a0
17:01:15     INFO -     r15 = 0x000000011ef6de80   rip = 0x0000000100a6791f
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO -  9  libnss3.dylib!_pt_root [ptthread.c:a1e5043844f2 : 216 + 0x3]
17:01:15     INFO -     rbx = 0x000000011f1bada0   rbp = 0x000000011f980ef0
17:01:15     INFO -     rsp = 0x000000011f980ed0   r12 = 0x000000000000d31b
17:01:15     INFO -     r13 = 0x00000000000008ff   r14 = 0x000000011f981000
17:01:15     INFO -     r15 = 0x0000000000000000   rip = 0x0000000100739069
17:01:15     INFO -     Found by: call frame info
17:01:15     INFO - 10  libsystem_pthread.dylib!_pthread_body + 0x83
17:01:15     INFO -     rbx = 0x000000011f981000   rbp = 0x000000011f980f10
17:01:15     INFO -     rsp = 0x000000011f980f00   r12 = 0x000000000000d31b
17:01:15     INFO -     r13 = 0x00000000000008ff   r14 = 0x000000011f1bada0
17:01:15     INFO -     r15 = 0x0000000100738f50   rip = 0x00007fff8210405a
17:01:15     INFO -     Found by: call frame info
Severity: normal → critical
Component: Marionette → Safe Browsing
Flags: needinfo?(francois)
Keywords: crash
Product: Testing → Toolkit
Version: Version 3 → unspecified
Henry, does this look like it may be related to the recent thread changes?
Flags: needinfo?(francois) → needinfo?(hchang)
Priority: -- → P1
I cannot certainly tell but there seems to be a concurrent access on
the non-thread-safe object mTableFreshness [1][2]. I'll try to fix this
first to see if this crash still happens.

[1] http://searchfox.org/mozilla-central/rev/eace920a0372051a11d8d275bd9b8f14f3024ecd/toolkit/components/url-classifier/Classifier.cpp#1282
[2] http://searchfox.org/mozilla-central/rev/eace920a0372051a11d8d275bd9b8f14f3024ecd/toolkit/components/url-classifier/Classifier.cpp#1188
Flags: needinfo?(hchang)
Assignee: nobody → hchang
Attachment #8856577 - Flags: review?(francois)
Comment on attachment 8856577 [details]
Bug 1354968 - Avoid concurrent access of mTableRefreshness.

https://reviewboard.mozilla.org/r/128532/#review131556

::: toolkit/components/url-classifier/Classifier.h:217
(Diff revision 3)
>    nsTArray<LookupCache*> mLookupCaches; // For query only.
>    nsTArray<nsCString> mActiveTablesCache;
>    uint32_t mHashKey;
>    // Stores the last time a given table was updated (seconds).
>    TableFreshnessMap mTableFreshness;
> +  TableFreshnessMap mTableFreshnessForUpdate;

Maybe we should call it `mNewTableFreshness` to match `mNewLookupCaches`.

Also, an end of line comment like `// for the update thread only` would be good.

::: toolkit/components/url-classifier/Classifier.cpp:624
(Diff revision 3)
>    for (auto c: mNewLookupCaches) {
>      delete c;
>    }
>    mNewLookupCaches.Clear();
>  
> +  // Reset mTableFreshnessForUpdate.

nit: that comment isn't really necessary
Attachment #8856577 - Flags: review?(francois) → review+
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7e8b84e2559
Avoid concurrent access of mTableRefreshness. r=francois
https://hg.mozilla.org/mozilla-central/rev/d7e8b84e2559
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: