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: