49813 - "threadsafe" nsDirectoryIndexStream uses nsDirEnumeratorUnix triggering "not thread-safe" assertion

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Randell Jesup [:jesup] (needinfo me)]

Got this assertion when browsing to file:///tmp on FreeBSD 3.4.  Build was
a fresh pull and clobber 8/21/00.  Probably related to bug 47356.

Breakpoint 2, nsDebug::Assertion (
    aStr=0x281eab80 "nsDirEnumeratorUnix not thread-safe", 
    aExpr=0x281f5ca0 "owningThread == NS_CurrentThread()", 
    aFile=0x281f5c7d "nsDebug.cpp", aLine=487) at nsDebug.cpp:191
(gdb) bt
#0  nsDebug::Assertion (
    aStr=0x281eab80 "nsDirEnumeratorUnix not thread-safe", 
    aExpr=0x281f5ca0 "owningThread == NS_CurrentThread()", 
    aFile=0x281f5c7d "nsDebug.cpp", aLine=487) at nsDebug.cpp:191
#1  0x281972f8 in NS_CheckThreadSafe (owningThread=0x8767600, 
    msg=0x281eab80 "nsDirEnumeratorUnix not thread-safe")
    at nsDebug.cpp:487
#2  0x28171c03 in nsDirEnumeratorUnix::Release (this=0x8f24220)
    at nsLocalFileUnix.cpp:150
#3  0x805eb57 in nsCOMPtr<nsISimpleEnumerator>::~nsCOMPtr (
    this=0x90c5f64, __in_chrg=2) at ../../dist/include/nsCOMPtr.h:489
#4  0x28749ce3 in nsDirectoryIndexStream::~nsDirectoryIndexStream (
    this=0x90c5f00, __in_chrg=3) at nsDirectoryIndexStream.cpp:92
#5  0x28749f0b in nsDirectoryIndexStream::Release (this=0x90c5f00)
    at nsDirectoryIndexStream.cpp:114
#6  0x805b4c9 in nsCOMPtr<nsIInputStream>::assign_assuming_AddRef (
    this=0x8bd8858, newPtr=0x0) at ../../dist/include/nsCOMPtr.h:471
#7  0x281cbab5 in nsCOMPtr<nsIInputStream>::assign_with_AddRef (
    this=0x8bd8858, rawPtr=0x0)
    at ../../../../dist/include/nsCOMPtr.h:848
#8  0x281cbced in nsCOMPtr<nsIInputStream>::operator= (this=0x8bd8858, 
    rhs=0x0) at ../../../../dist/include/nsCOMPtr.h:583
#9  0x28738b6c in nsFileTransport::Process (this=0x8bd8800)
    at nsFileTransport.cpp:512
#10 0x287381bd in nsFileTransport::Run (this=0x8bd8800)
    at nsFileTransport.cpp:360
#11 0x2819088b in nsThreadPoolRunnable::Run (this=0x8258680)
    at nsThread.cpp:689
#12 0x2818e4cd in nsThread::Main (arg=0x81e4de0) at nsThread.cpp:84
#13 0x282fd271 in _pt_root (arg=0x8238f80) at ptthread.c:198
#14 0x28346a5e in _thread_start () from /usr/lib/libc_r.so.3
#15 0x0 in ?? ()
(gdb)

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

After checking a little more on non-thread-safe issues, I'm raising this to 
Major.  thread-safety issues could cause crashes and/or dataloss.

Comment 2

[Blake Ross]

Missed the 0.9.3 train.

Comment 3

[Judson Valeski]

and 0.9.4

Comment 4

[rpotts (gone)]

->0.9.6

Comment 5

[rpotts (gone)]

-> 1.0

Comment 6

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 7

[Hixie (not reading bugmail)]

Moving all threading bugs to XPCOM. See bug 160356.

Comment 8

[Vedran Miletic]

retargeting

Comment 9

[timeless]

Attachment: fix caller

Comment 10

[timeless]

I attached one way that should resolve this problem

Comment 11

[Darin Fisher]

Comment on attachment 134506 [details] [diff] [review]
fix caller

i think it'd be better to not store mDir.  it doesn't even seem to be used for
anything past Init(), so i think you can just stop storing it :-)

Comment 12

[Bradley Baetz (:bbaetz)]

I have this vague recollection that I needed this for something. this ties into
the THREADSAFE_I18N stuff too, of course.

Comment 13

[timeless]

Attachment: fix caller, skip the useless member

Comment 14

[Darin Fisher]

Comment on attachment 134810 [details] [diff] [review]
fix caller, skip the useless member

hmm... if we are no longer saving mDir, then is the Clone really necessary?

Comment 15

[timeless]

darin: well, the "interface" doesn't specify whether the nsIFile it takes is
usable. it turns out that you "fixed" the caller (nsFileChannel) by having it
create a new nsIFile which it passed to nsDirectoryIndexStream. Given the way
the code is today, I'll drop the clone and add comments to the .h indicating
that the nsIFile will be used on its calling thread.

Comment 16

[Darin Fisher]

Comment on attachment 134810 [details] [diff] [review]
fix caller, skip the useless member

r+sr=darin w/ dir cloning dropped.

Comment 17

[timeless]

checked in

Comment 18

[benc]

VERIFIED:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsDirectoryIndexStream.cpp&branch=&root=/cvsroot&subdir=mozilla/netwerk/base/src&command=DIFF_FRAMESET&rev1=1.27&rev2=1.28

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsDirectoryIndexStream.h&branch=&root=/cvsroot&subdir=mozilla/netwerk/base/src&command=DIFF_FRAMESET&rev1=1.8&rev2=1.9