252289 - M18a2 topcrash in safefileoutputstream [@ nsSafeFileOutputStream::Init]

Status: VERIFIED FIXED

(Core :: Networking: File, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsSafeFileOutputStream%3A%3AInit

stacks point to:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/base/src/nsFileStreams.cpp&rev=1.64&root=/cvsroot&mark=503#503

Called from
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/cookie/src/nsCookieService.cpp&rev=1.38&mark=1039#1039

Looks like mCookieFile is null. Should not happen, unless there is no profile.
But when is this code called without a profile?
anyway, a null check somewhere should fix this.

Comment 1

[[e-mail]]

Attachment: nullcheck file

check in the stream init, to make it consistent with nsFileOutputStream.

Comment 2

[[e-mail]]

Attachment: nullcheck mCookieFile too

cookies is called without a profile at startup, in seamonkey anyway (not sure
about fx). we initialize it for some silly thing before the profile manager
comes up. i can't remember why, but i hate it. i don't know why we haven't seen
this crash ourselves.

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 153828 [details] [diff] [review]
nullcheck file

>-        NS_ENSURE_STATE(mTargetFile);
Hmm, maybe you should leave that check there. It never hurts to be sure. A bad
consumer can call Finish() even when Init() failed.

Other checks is good. r=mvl

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 153829 [details] [diff] [review]
nullcheck mCookieFile too

>+  NS_ENSURE_ARG(mCookieFile);
NS_ENSURE_STATE might be more appropiate. It isn't an arg.
Also, if you know that it will warn on startup because cookies start when there
is no profile, is it worth warning?

Comment 5

[[e-mail]]

you're right, it's not. i'll make it an |if| check instead.

Comment 6

[Darin Fisher]

please post one final patch for this bug, thanks!

Comment 7

[Darin Fisher]

Comment on attachment 153828 [details] [diff] [review]
nullcheck file

i agree with mvl, keep the NS_ENSURE_STATE(mTargetFile)

shouldn't mTargetFileExists be initialized in the ctor?

Comment 8

[Darin Fisher]

Comment on attachment 153829 [details] [diff] [review]
nullcheck mCookieFile too

minus in favor of NS_ENSURE_STATE as suggested by mvl.

Comment 9

[Jay Patel [:jay]]

Adding topcrash info for tracking, this is a topcrasher with Mozilla 1.8 alpha2.

Comment 10

[[e-mail]]

Attachment: here we go

Comment 11

[Darin Fisher]

Comment on attachment 153931 [details] [diff] [review]
here we go

sr=darin

Comment 12

[benc]

V/fixed, per lxr.