572011 - Crash @ nsDiskCacheStreamIO::FlushBufferToFile

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[Ricardo Maçãs [:Ricmacas]]

Hello, one user intially reported this crash on SUMO Twitter

3.6.3 http://crash-stats.mozilla.com/report/index/219cdbde-e406-4f0f-8c60-00ccc2100612

But we asked the user to upgrade to 3.6.4 beta 7, and the crashes have now this signature (presented in summary):

3.6.4 http://crash-stats.mozilla.com/report/index/c21105ba-e160-4a63-92a1-b19732100613

I'll have more information when possible.
All crash reports are in clean profiles.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

0  	ntdll.dll  	strstr  	
1 	xul.dll 	nsDiskCacheStreamIO::FlushBufferToFile 	netwerk/cache/src/nsDiskCacheStreams.cpp:731
2 	xul.dll 	nsDiskCacheStreamIO::Write 	netwerk/cache/src/nsDiskCacheStreams.cpp:587
3 	xul.dll 	nsDiskCacheOutputStream::Write 	netwerk/cache/src/nsDiskCacheStreams.cpp:257
4 	xul.dll 	nsCacheEntryDescriptor::nsOutputStreamWrapper::Write 	netwerk/cache/src/nsCacheEntryDescriptor.cpp:647
5 	xul.dll 	nsInputStreamTee::TeeSegment 	xpcom/io/nsInputStreamTee.cpp:80
6 	xul.dll 	nsInputStreamTee::WriteSegmentFun 	xpcom/io/nsInputStreamTee.cpp:109
7 	xul.dll 	nsPipeInputStream::ReadSegments 	xpcom/io/nsPipe3.cpp:799
8 	xul.dll 	nsInputStreamTee::ReadSegments 	xpcom/io/nsInputStreamTee.cpp:156
9 	xul.dll 	nsStreamLoader::OnDataAvailable 	netwerk/base/src/nsStreamLoader.cpp:180
10 	xul.dll 	nsStreamListenerTee::OnDataAvailable 	netwerk/base/src/nsStreamListenerTee.cpp:108
11 	xul.dll 	nsHttpChannel::OnDataAvailable 	netwerk/protocol/http/src/nsHttpChannel.cpp:5388
12 	xul.dll 	nsInputStreamPump::OnStateTransfer 	netwerk/base/src/nsInputStreamPump.cpp:508
13 	xul.dll 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:398
14 	xul.dll 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:191
15 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:527
16 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:142

Comment 2

[Ricardo Maçãs [:Ricmacas]]

Not a result of locked or corrupted files, because clean install didn't fix this issue.

Comment 3

[Tobias (:Tobbi) Markus]

(In reply to comment #2)
> Not a result of locked or corrupted files, because clean install didn't fix
> this issue.

What about a new Firefox profile and manually copying important data over?

Comment 4

[Ricardo Maçãs [:Ricmacas]]

Tobbi, 
> All crash reports are in clean profiles.
Besides, if you checked the crash reports, you could clearly see they are clean.
This user has clean profile, in a clean install.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Looks like this crash is 1.9.1 and 1.9.2 only. There are no crashes listed for trunk:

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29&date=06%2F15%2F2010%2006%3A25%3A38&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=1&signature=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29

Comment 6

[chris hofmann]

the 3.6.4 and 3.6.4 crash don't look like the share much common code on the stacks.  they could be two entirely different problems, but tickled by some underlying problem(s).   

The  nsDiskCacheStreamIO::FlushBufferToFile might have 6 different associated signatures

nsDiskCacheStreamIO::FlushBufferToFile signature breakdown
signature distribution
       6
signature list
 475 strstr | nsDiskCacheStreamIO::FlushBufferToFile()
  16 strstr | @0x1318d523 | nsDiskCacheStreamIO::FlushBufferToFile()
   9 nsDiskCacheStreamIO::FlushBufferToFile()
   1 strstr | @0x1310cfd3 | nsDiskCacheStreamIO::FlushBufferToFile()
   1 strlen | @0x7d50d53 | nsDiskCacheStreamIO::FlushBufferToFile()
   1 strlen | @0x4a40d53 | nsDiskCacheStreamIO::FlushBufferToFile()

Correlation to startup or time of session
503 total crashes for nsDiskCacheStreamIO::FlushBufferToFile on 20100614-crashdata.csv
138 startup crashes inside 30 sec.
228 startup crashes inside 3 min.
141 repeated crashes inside 3 min. of last crash

but they all seem to be happening on winXP

os breakdown
nsDiskCacheStreamIO::FlushBufferToFileTotal 9
Win5.1  1.00
Win6.0  0.00
Win6.1  0.00

and the problem stretches back across many releases.

checking --- nsDiskCacheStreamIO::FlushBufferToFile 20100614-crashdata.csv
found in: 3.6.3 3.5.9 3.6.4 3.0.19 3.6 3.6.2 3.0.4 3.5.1 3.0 3.5.7 3.5.5 3.5.3 3.5.2 3.5 3.0.7 3.0.11 3.0.10 3.0.1
release total-crashes
              nsDiskCacheStreamIO::FlushBufferToFile crashes
                         pct.
all     408812  503     0.00123039
3.6.3   270109  369     0.00136612
3.5.9   33612   52      0.00154707
3.6.4   50079   29      0.000579085
3.0.19  12129   16      0.00131915
3.6     11172   10      0.000895095
3.6.2   3238    5       0.00154416
3.0.4   541     5       0.00924214
3.5.1   439     3       0.00683371
3.0     1120    3       0.00267857
...


If the profiles are clean, one thing I'd suggest checking is for disk corruption on the systems where people are hitting this, and we should try to isolate to possible file system problems.

Comment 7

[Ricardo Maçãs [:Ricmacas]]

Well, both crash [@ strstr], the rest is unrelated...

Comment 8

[Ricardo Maçãs [:Ricmacas]]

User reports crashes in plugincontainer.exe in new profile and clean install, more info in a sec.

Comment 9

[Ricardo Maçãs [:Ricmacas]]

User claims he did a disk check and everything's fine.

Comment 10

[Ricardo Maçãs [:Ricmacas]]

He has scanned his system using Avira, Malware Bytes and Super Anti-Spyware

Comment 11

[Wayne Mery (:wsmwk)]

seen also in thunderbird - example bp-f1e0c3ae-f635-42db-9372-483252100625. some thunderbird also has stack variations:
strlen | @0x1910ae8 | nsDiskCacheStreamIO::FlushBufferToFile()
strlen | @0x2210ae8 | nsDiskCacheStreamIO::FlushBufferToFile()
strlen | @0x2ae0ae8 | nsDiskCacheStreamIO::FlushBufferToFile()

Odd, no thunderbird crashes with strlstr... sig prior to calendar date   Jun 07, 2010 13:18 for any version, even though there is a 3.0 crash bp-b0d17e78-09c7-480d-9880-db8c12100607 (and it's the only crash for that user - no other versions appear for that OS)

mod summary - socorro prefers  strstr | nsDiskCacheStreamIO::FlushBufferToFile()

Comment 12

[Scoobidiver (away)]

It's #26 top browser crasher in 10.0b6 while only #137 in 9.0.1.

Comment 13

[Scoobidiver (away)]

It's #61 top browser crasher in 10.0.1, #28 in 11.0b2, and #39 in 12.0a2.
It might be related to bug 727097.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29

Comment 14

[Wayne Mery (:wsmwk)]

nsDiskCacheStreamIO::FlushBufferToFile() is rare for thunderbird - roughly a dozen in a week

Comment 15

[Scoobidiver (away)]

It's now #4 top browser crasher in 10.0.2 and #6 in 11.0.

It is correlated to Yandex Toolbar and Спутник @Mail.Ru:
     38% (345/904) vs.   7% (4472/65399) {37964A3C-4EE8-47b1-8321-34DE2C39BA4D} (Спутник @Mail.Ru)
     38% (348/904) vs.   8% (5036/65399) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)

It is also correlated to Windows Based Wireless Networks:
     34% (309/904) vs.   6% (3695/65399) wlanapi.dll

Comment 16

[Robert Kaiser]

This stays constantly at high ranks. #7 in yesterday's data for 12.*

Any chance someone can find out what's going on there?

Comment 17

[Scoobidiver (away)]

There's a spike in crashes that started around 15.0a1/20120426 (10 crashes per buildID before, 20 crashes per buildID after):
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A14.0a1&version=Firefox%3A15.0a1&query_search=signature&query_type=contains&reason_type=contains&range_value=28&range_unit=days&do_query=1&signature=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29

Comment 18

[Sheila Mooney]

Now it's at #5 on 12.

Comment 19

[Robert Kaiser]

And this is #3 on 13 Beta in yesterday's data. Any chance we can do something about it?

Comment 20

[Josh Aas]

Michal - Can you take a look at this?

Comment 21

[Josh Aas]

Ted suggested that it might be a problem with LSP's based on the stack trace.

Comment 22

[(not currently active) Ted Mielczarek]

It's possible that these are not all the same crash, but I grabbed the minidump from https://crash-stats.mozilla.com/report/index/90b4f9bf-b59b-4671-af96-d61122120613 . WinDBG gives me the exact same stack as Breakpad. Running minidump_stackwalk locally shows me:
Thread 25 (crashed)
 0  0x16c26f    eip = 0x0016c26f   esp = 0x084ffde0   ebp = 0x084ffe10   ebx = 0xffe92500
    esi = 0x00176b01   edi = 0x00176b00   eax = 0x00000000   ecx = 0x07a6e000
    edx = 0xffffffff   efl = 0x00010246
    Found by: given as instruction pointer in context
1  0x15d30c
    eip = 0x0015d30d   esp = 0x084ffe18   ebp = 0x084ffe30
    Found by: previous frame's frame pointer

 2  xul.dll!nsDiskCacheStreamIO::FlushBufferToFile() [nsDiskCacheStreams.cpp:2b6
43ea8edf9 : 791 + 0xe]
    eip = 0x012a1be6   esp = 0x084ffe38   ebp = 0x084ffe5c
    Found by: previous frame's frame pointer

So it's not as though Breakpad is resorting to stack scanning here, it's using frame pointers. I also ran another tool I have that scans the stack looking for anything that looks like a return address (http://hg.mozilla.org/users/tmielczarek_mozilla.com/dump-lookup/), and it doesn't show anything more useful on the stack above FlushBufferToFile.

I also looked at the list of modules that we ever loaded in the process, and there didn't seem to be anything at the address where we're crashing.

Comment 23

[Sheila Mooney]

This is #13 top crash on the list for 13.0.1.

Comment 24

[Robert Kaiser]

This seems to be spiking again. I'm pretty sure it's connected to some third-party software as it's rising on multiple versions at the same time, but I wonder if there's something we can do here.

Comment 25

[Alfred Kayser]

I have possibly some options to reduce or remove this crasher...

Comment 26

[Ludovic Hirlimann [:Usul]]

(In reply to Alfred Kayser from comment #25)
> I have possibly some options to reduce or remove this crasher...

Cool !!!

Comment 27

[Alfred Kayser]

1. Remove 'offset' from nsDiskCacheStream::GetOutputStream (it is always zero). This simplifies the code, particular around the buffer handling.
2. Use buffer if and only aslong as the data fits in cacheblocks, otherwise directly write to the file (skipping the buffer) (the write to the file is buffered itself). This reduces the need for the buffer and chances of crashes.
3. Merge nsDiskCacheOutputStream with nsDiskCacheStreamIO: less malloc's and less code overall: reduces risk of crashing.

Comment 28

[Marcia Knous [:marcia]]

Here are some correlations from Firefox 17 that implicate Yandex in the crash on the Firefox side:

nsDiskCacheStreamIO::FlushBufferToFile()|EXCEPTION_ACCESS_VIOLATION_READ (228 crashes)
     43% (98/228) vs.   4% (1843/43394) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)
     42% (96/228) vs.   4% (1728/43394) [e-mail]
     37% (84/228) vs.   2% (947/43394) {37964A3C-4EE8-47b1-8321-34DE2C39BA4D}
     15% (35/228) vs.   0% (106/43394) {6236BA26-C117-4007-928C-DE0716C7FA78} (Chameleon Bob: layouts for Facebook, https://addons.mozilla.org/addon/11584)
     15% (35/228) vs.   0% (108/43394) {6236BA26-C117-4007-928C-DE0716C7FA48}
     15% (35/228) vs.   0% (109/43394) {6236BA26-C117-4007-928C-DE0716C7FA58}
     15% (35/228) vs.   0% (112/43394) {6236BA26-C117-4007-928C-DE0716C7FA68} (Express Tab, https://addons.mozilla.org/addon/14584)
     15% (35/228) vs.   0% (120/43394) {6236BA26-C117-4007-928C-DE0716C7FA38}
     15% (35/228) vs.   0% (153/43394) {4933189D-C7F7-4C6E-834B-A29F087BFD23}
     12% (28/228) vs.   2% (889/43394) [e-mail] (Java Quick Starter, http://java.sun.com/javase/downloads/)
     10% (23/228) vs.   0% (30/43394) [e-mail]
      9% (21/228) vs.   0% (25/43394) [e-mail]
      6% (14/228) vs.   0% (18/43394) [e-mail]
      6% (14/228) vs.   1% (399/43394) {dfefbe51-ca52-484b-adf0-6b158b05262d}
     98% (223/228) vs.  93% (40195/43394) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)
      6% (14/228) vs.   1% (437/43394) {5911488E-9D1E-40ec-8CBB-06B231CC153F}
      6% (14/228) vs.   1% (476/43394) [e-mail]

Comment 29

[Robert Kaiser]

The nsDiskCacheStreamIO::FlushBufferToFile() signature is #21 on 18.0b3 and up to #11 on 17.0.1 release.

As the Yandex Bar add-on is completely JS-based (no binary code), any crash it triggers is very probably a problem in our code and we should investigate that, esp. with how much-used this add-on is in Russia.

sfink, you have done work with debugging crashes with this add-on in bug 770238, can you help here as well?

Comment 30

[Scoobidiver (away)]

The fix in bug 814010 will likely fix this bug. It seems risky to uplift it in branches.

Comment 31

[Alex Keybl [:akeybl]]

Longstanding issue, we'll look into uplifting bug 814010 for FF19.

Comment 32

[Alfred Kayser]

Attachment: Don't crash on write of empty buffer

Still causing crashes (although less than before).

The crash is at PR_Write with access to 0x0. As mFD cannot be zero at that point (see code before), the other suspect is mBuffer:
   if (PR_Write(mFD, mBuffer, mStreamEnd) != (int32_t)mStreamEnd) {

mBuffer could be zero when there is no buffer allocated (either failed to or first chunk is directly greater than 16k). In both cases, mStreamEnd will be zero, as there is nothing in the buffer.

So to prevent PR_Write crashing, a check around it with mBuffer != null and mStreamEnd > 0 will help preventing to crash here.

Comment 33

[Alfred Kayser]

Try: https://mail.google.com/mail/u/0/#inbox/13c290d5f4f527b2

Comment 34

[Michal Novotny [:michal]]

Comment on attachment 700937 [details] [diff] [review]
Don't crash on write of empty buffer

First, I think that this crash isn't caused by mBuffer being null. See comment #6 in bug #752266. Also see discussion in bug #801394. BTW, this bug is also Windows only problem.

Anyway, I agree that we should make sure that we don't pass null to PR_Write(). But I don't think that we should try to survive malloc() failure, because if we are out of memory the application will crash sooner or later. So I propose to change malloc()/realloc() with infallible moz_xmalloc()/moz_xrealloc() and to place a check before PR_Write() that aborts Firefox (even in release build) in case mBuffer is null, something like this:

  if (!mBuffer) {
    NS_RUNTIMEABORT("Fix me!");
  }

Comment 35

[Alfred Kayser]

There can be multiple issues that can cause PR_Write to crash:
1. Failed to allocate mBuffer, because of lack of memory...
2. No mBuffer allocated because first part is > 16K, so the buffer allocate stage is skipped.
3. No mBuffr allocated because count = 0.

Number 1 is crisis-mode, but should just return OUT_OF_MEMORY from the write call, so that stream writer can handle it.

Number 2 and 3 should be protected within the code as in patch https://bugzilla.mozilla.org/attachment.cgi?id=700937.
Number 2 will happen when chunks larger than 16K are written in the first call to Write.

So, while number 1 is memory or virus or such related, number 2 and 3 are to be fixed by this patch.

Comment 36

[Scoobidiver (away)]

Depending on the crash signatures, 43% or 32% of crashes occur within one minute. It doesn't qualify this bug for the startupcrash whiteboard that requires more than 50% of crashes.

Comment 37

[Robert Kaiser]

Hmm, for Firefox Aurora, I did see the startup "rocket" symbol on it, that's what I told Alex in the channel meeting and he marked it that way.
I now see that it barely qualifies there: https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2013-01-25&signature=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29&version=Firefox%3A20.0a2

Comment 38

[Michal Novotny [:michal]]

(In reply to Alfred Kayser from comment #35)
> Number 1 is crisis-mode, but should just return OUT_OF_MEMORY from the write
> call, so that stream writer can handle it.

As I wrote, use infallible memory allocators and don't try to handle OOM.

The rest of the patch is OK.

Comment 39

[Alfred Kayser]

Attachment: V2: Use moz_xmalloc

Comment 40

[Alfred Kayser]

https://tbpl.mozilla.org/?tree=Try&rev=3b93bf1943d2

Comment 41

[Alfred Kayser]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9c36d39f7bc

Comment 42

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d9c36d39f7bc

Comment 43

[Alfred Kayser]

This needs to be applied to Aurora/Mozilla20 as it fixes a potential crash issue caused by the patch of bug 572011 which is applied to Mozilla20.

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+1]]

If tracking gets denied we may have to ask for approval of the patch. But the target milestone is still 21.

Comment 45

[Scoobidiver (away)]

(In reply to Alfred Kayser from comment #43)
> This needs to be applied to Aurora/Mozilla20 as it fixes a potential crash
> issue caused by the patch of bug 572011 which is applied to Mozilla20.
Here it's already bug 572011, a long standing issue. Are you sure you posted in the right bug?

Comment 46

[Alfred Kayser]

This is the right bug with the patch that needs to go into Mozilla20.
I meant to refer to bug 814010. See comment 19 in there.

Comment 47

[Robert Kaiser]

BTW, "Target Milestone" means when it is supposed to be or was fixed on mozilla-central. If you want to indicate what other versions are affected, that's what the status-* flags are about (setting that for 20), tracking-* means that we should track this for a given train, if it's just about getting a patch uplifted, all that's needed is the relevant approval-* flag(s) on the patch (and switching the fitting status flag to fixed once the uplift lands).

Comment 48

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking - go ahead with nominating the patch for aurora uplift.

Comment 49

[Lukas Blakk [:lsblakk] use ?needinfo]

We'll be merging Aurora to the Beta channel next Tuesday so an uplift nomination and landing would be great to have before then so this crash doesn't hit our larger audience.

Comment 50

[Jason Duell]

Comment on attachment 709359 [details] [diff] [review]
V2: Use moz_xmalloc

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 814010
User impact if declined: crash
Testing completed (on m-c, etc.): on m-c for a week now.
Risk to taking this patch (and alternatives if risky):  very low risk IMO.  Just a couple if (null) checks and switch to use moz_xmalloc from malloc.  Trivial.
String or UUID changes made by this patch: none.

Comment 51

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 709359 [details] [diff] [review]
V2: Use moz_xmalloc

Thank you - please go ahead with this low risk uplift.

Comment 52

[Jason Duell]

https://hg.mozilla.org/releases/mozilla-aurora/rev/55c50cc02d40

Comment 53

[Ioana (away)]

No post-fix crashes for [@ nsDiskCacheStreamIO::FlushBufferToFile()] and [@ strstr]:
https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29&reason_type=contains&date=03%2F14%2F2013%2009%3A30%3A48&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=strstr&reason_type=contains&date=03%2F14%2F2013%2009%3A30%3A49&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=strstr

6 post-fix crashes for [@ strstr | nsDiskCacheStreamIO::FlushBufferToFile()]:
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A22.0a1&version=Firefox%3A21.0a2&version=Firefox%3A20.0b4&version=Firefox%3A20.0b5&version=Firefox%3A20.0b3&version=Firefox%3A20.0b2&version=Firefox%3A20.0b1&query_search=signature&query_type=contains&query=strstr%20%7C%20nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29&reason_type=contains&date=03%2F14%2F2013%2009%3A25%3A28&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=strstr%20%7C%20nsDiskCacheStreamIO%3A%3AFlushBufferToFile%28%29

Definitely not a top crash anymore.

Comment 54

[:Irving Reid (No longer working on Firefox)]

I took a quick look at the six post-fix crashes, and I'm guessing that the problem is that mFD is corrupted somehow - at http://hg.mozilla.org/releases/mozilla-beta/annotate/64a66423dbd3/netwerk/cache/nsDiskCacheStreams.cpp#l706, FlushBufferToFile() calls PR_Write, which calls through a function pointer mFD->methods->write. If that's pointing somewhere odd, that would explain why strstr (or other seemingly unrelated symbols) shows up at the top of the stack.

The most likely candidate for corrupt mFD would be use-after-free (most likely in the form of use-after-close), but based on a quick review I'm not seeing an obvious place where that would happen.