Closed Bug 1043843 Opened 10 years ago Closed 10 years ago

Intermittent test_fs_createFile.html,test_sync_worker.html | application crashed [@ mozPersonalDictionary::Release()]

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34
Tracking Flags:
Tracking Status
firefox32 --- unaffected
firefox33 --- fixed
firefox34 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.1 --- fixed

People

(Reporter: cbook, Assigned: xyuan)

References

(
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

threadsafe.patch
4.19 KB, patch
xyuan
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Android 2.3 Armv6 Emulator mozilla-inbound opt test mochitest-7 on 2014-07-25 00:32:16 PDT for push 6624622f7886 slave: tst-linux64-spot-363 https://tbpl.mozilla.org/php/getParsedLog.php?id=44581870&tree=Mozilla-Inbound PROCESS-CRASH | /tests/dom/devicestorage/test/test_fs_createFile.html | application crashed [@ mozPersonalDictionary::Release()] 00:54:05 INFO - CPU: arm 00:54:05 INFO - 0 CPUs 00:54:05 INFO - 00:54:05 INFO - Crash reason: SIGSEGV 00:54:05 INFO - Crash address: 0x0 00:54:05 INFO - 00:54:05 INFO - Thread 45 (crashed) 00:54:05 INFO - 0 libxul.so!mozPersonalDictionary::Release() [mozPersonalDictionary.cpp:6624622f7886 : 36 + 0x8] 00:54:05 INFO - r4 = 0x52e58360 r5 = 0x46f3c100 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffd90 lr = 0x49b79b2c pc = 0x4c929124 00:54:05 INFO - Found by: given as instruction pointer in context 00:54:05 INFO - 1 libxul.so!mozilla::RefPtr<mozilla::SharedThreadPool>::~RefPtr() + 0x1e 00:54:05 INFO - r4 = 0x55198f84 r5 = 0x00000000 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffda0 pc = 0x4c427b94 00:54:05 INFO - Found by: call frame info 00:54:05 INFO - 2 libxul.so!mozilla::dom::CreateFileTask::~CreateFileTask() [nsCOMPtr.h:6624622f7886 : 466 + 0xe] 00:54:05 INFO - r4 = 0x55198f40 r5 = 0x00000000 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffda8 pc = 0x4ce83350 00:54:05 INFO - Found by: call frame info 00:54:05 INFO - 3 libxul.so!mozilla::dom::CreateFileTask::~CreateFileTask() [CreateFileTask.cpp:6624622f7886 : 90 + 0x2] 00:54:05 INFO - r4 = 0x55198f40 r5 = 0x00000000 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffdb0 pc = 0x4ce83390 00:54:05 INFO - Found by: call frame info 00:54:05 INFO - 4 libxul.so!PipUIContext::Release() [nsNSSComponent.cpp:6624622f7886 : 1702 + 0xe] 00:54:05 INFO - r4 = 0x55198f40 r5 = 0x00000000 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffdb8 pc = 0x4c487570 00:54:05 INFO - Found by: call frame info 00:54:05 INFO - 5 libxul.so!mozilla::RefPtr<mozilla::SharedThreadPool>::~RefPtr() + 0x1e 00:54:05 INFO - r4 = 0x53bffdd8 r5 = 0x00000000 r6 = 0x52d54074 r7 = 0x53bffdd0 00:54:05 INFO - r8 = 0x00000000 r9 = 0x00000000 r10 = 0x0010f1cc fp = 0x52d54074 00:54:05 INFO - sp = 0x53bffdc8 pc = 0x4c427b94
Summary: Intermittent est_fs_createFile.html | application crashed [@ mozPersonalDictionary::Release()] → Intermittent test_fs_createFile.html | application crashed [@ mozPersonalDictionary::Release()]
Possibly a regression from bug 1036500? Though that landed about a day earlier, so that seems unlikely.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #13) > Possibly a regression from bug 1036500? Though that landed about a day > earlier, so that seems unlikely. No, it should not be a regression from bug 1036500, as that bug didn't touch any code with this bug. But it seems the cause is the same as that of bug 1036500. Both of them are caused by Bug 827823，that changed DOMFile to non-thread-safe.
https://tbpl.mozilla.org/php/getParsedLog.php?id=44910339&tree=Mozilla-Inbound
Summary: Intermittent test_fs_createFile.html | application crashed [@ mozPersonalDictionary::Release()] → Intermittent test_fs_createFile.html,test_sync_worker.html | application crashed [@ mozPersonalDictionary::Release()]
Attached patch threadsafe.patch (obsolete) — — Splinter Review
Bug 827823 made nsIDOMBlob not thread-safe. CreateFileTask has a nsIDOMBlob member of mBlobData, which is accessed on main thread, but may be released on worker thread. It seems the non thread-safe releasing of mBlobData causes this crash bug. The patch ensure that the mBlobData will be released on main thread after being used.
Assignee: nobody → xyuan
Status: NEW → ASSIGNED
Comment on attachment 8472902 [details] [diff] [review] threadsafe.patch Could you help review this? refer to comment 34 about the patch changes.
Attachment #8472902 - Flags: review?(dhylands)
Comment on attachment 8472902 [details] [diff] [review] threadsafe.patch Review of attachment 8472902 [details] [diff] [review]: ----------------------------------------------------------------- This looks reasonable to me. Just a couple issues to address. r=me with issues addressed. ::: dom/filesystem/CreateFileTask.cpp @@ +41,5 @@ > + if (FileSystemUtils::IsParentProcess()) { > + nsresult rv = aBlobData->GetInternalStream(getter_AddRefs(mBlobStream)); > + NS_WARN_IF(NS_FAILED(rv)); > + } else { > + mBlobData = aBlobData; This was already done above on line 34. Should line 34 be removed? Or should this line? @@ +77,5 @@ > return; > } > > BlobParent* bp = static_cast<BlobParent*>(static_cast<PBlobParent*>(data)); > + nsRefPtr<nsIDOMBlob> blobData = bp->GetBlob(); Since this is nsIDOMBlob, I think you should be using nsCOMPtr
Attachment #8472902 - Flags: review?(dhylands) → review+
Thanks for your help. (In reply to Dave Hylands [:dhylands] from comment #37) > Comment on attachment 8472902 [details] [diff] [review] > threadsafe.patch > > Review of attachment 8472902 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks reasonable to me. Just a couple issues to address. > > r=me with issues addressed. > > ::: dom/filesystem/CreateFileTask.cpp > @@ +41,5 @@ > > + if (FileSystemUtils::IsParentProcess()) { > > + nsresult rv = aBlobData->GetInternalStream(getter_AddRefs(mBlobStream)); > > + NS_WARN_IF(NS_FAILED(rv)); > > + } else { > > + mBlobData = aBlobData; > > This was already done above on line 34. Should line 34 be removed? Or should > this line? Line 34 should be removed and I forgot that. > > @@ +77,5 @@ > > return; > > } > > > > BlobParent* bp = static_cast<BlobParent*>(static_cast<PBlobParent*>(data)); > > + nsRefPtr<nsIDOMBlob> blobData = bp->GetBlob(); > > Since this is nsIDOMBlob, I think you should be using nsCOMPtr Yes, I should use nsCOMPtr for holding interface.
Attached patch threadsafe.patch — — Splinter Review
Fixed according to comment 39 and update commit message to add review info. Try: https://tbpl.mozilla.org/?tree=Try&rev=ab07595a7eae https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ab07595a7eae
Attachment #8472902 - Attachment is obsolete: true
Attachment #8474347 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/f9d39c068ecd
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Please nominate this for Aurora when you get a chance.
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → fixed
status-firefox32: --- → unaffected
status-firefox33: --- → affected
status-firefox34: --- → fixed
status-firefox-esr24: --- → unaffected
status-firefox-esr31: --- → unaffected
Flags: needinfo?(xyuan)
Comment on attachment 8474347 [details] [diff] [review] threadsafe.patch Approval Request Comment [Feature/regressing bug #]: Bug 1043843 [User impact if declined]: Crashes when using DeviceStorage Filesytem API to create files. [Describe test coverage new/current, TBPL]:https://tbpl.mozilla.org/?tree=Try&rev=ab07595a7eae [Risks and why]: No risk. The patch made DOMBlob reference released a little earlier after being used and should not cause any fallout. [String/UUID change made/needed]: None.
Attachment #8474347 - Flags: approval-mozilla-aurora?
Flags: needinfo?(xyuan)
Attachment #8474347 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: