39373 - [prefs]JS on threads other than "main" must run within requests

Reporter

Description

•

24 years ago

Using the JS_{Begin,End,Suspend,Resume}Request API, and of course 
JS_SetContextThread.

I filed this bug against XPConnect, because that's the likliest path into the JS 
engine on non-main threads, but there could be others.  I'll lxr and enumerate 
the control flow cases here.  Any help welcome.

/be

Brendan Eich [:brendan]

Reporter

Comment 1

•

24 years ago

Assigning and fixing fields.

/be

Status: NEW → ASSIGNED

Depends on: 39321

OS: Windows 98 → All

Hardware: PC → All

Target Milestone: --- → M16

Brendan Eich [:brendan]

Reporter

Comment 2

•

24 years ago

This is a potential crash bug.  I can't assess the probability, but the cost is 
high, and risk=p*cost.

/be

Keywords: dogfood

Phil Peterson

Comment 3

•

24 years ago

What's the user impact of this bug? Also, chofmann says that if you can tell him
what would be on the stack, he can search through Talkback for occurrences.

Whiteboard: [NEED INFO]

Brendan Eich [:brendan]

Reporter

Comment 4

•

24 years ago

User impact is random crashes, probably around closing windows or unloading
documents (those actions run the JS GC -- but it also runs at other times).

Crashes would be in JS, but not in gc_root_marker or other jsgc.c functions (I
mention that function also because it has shown up in talkback, due to XBL GC
root set management bugs).  The crashes would rather be in the JS compiler
(jsparse.c) or interpreter (jsinterp.c, jsobj.c), or possibly in jsapi.c code.

I don't know why I added dogfood to this bug, I meant nsbeta2.  If this bug is
not happening often enough to show up on talkback or among daily build testers,
it could be marked nsbeta2-. I'll come up with a patch soon, to get some code
review.  That'll give you more choices about how to proceed.

/be

Phil Peterson

Comment 5

•

24 years ago

[dogfood-] [nsbeta2-] per brendan's suggestion. chofmann, if this is prominent
in talkback, please renominate.

Whiteboard: [NEED INFO] → [dogfood-] [nsbeta2-]

Brendan Eich [:brendan]

Reporter

Comment 6

•

24 years ago

Moving out, we simply aren't seeing crashes due to this bug.

/be

Target Milestone: M16 → M17

Brendan Eich [:brendan]

Reporter

Updated

•

24 years ago

Keywords: arch

Target Milestone: M17 → M18

Daniel Veditz [:dveditz]

Comment 7

•

24 years ago

Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the 
queries don't get screwed up

Keywords: nsbeta2

Brendan Eich [:brendan]

Reporter

Updated

•

24 years ago

Depends on: 50859

chris hofmann

Comment 8

•

24 years ago

don't know if this is related but...
here is a top crash with jsapi on the stack,
looks like some garbage collection might be going on, 
and lots of users commenting that the are exiting a mail message,
  closing down bookmarks, or exiting netscape...

nsCacheEntryChannel::~nsCacheEntryChannel
Stack Trace: 
nsCacheEntryChannel::~nsCacheEntryChannel       
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsCacheEntryChannel.cpp
nsCacheEntryChannel::`scalar deleting destructor'
nsDiskCacheRecordChannel::Release       
[d:\builds\seamonkey\mozilla\netwerk\cache\filecache\nsDiskCacheRecordChannel.cp
p
nsCOMPtr_base::~nsCOMPtr_base   
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp
nsHTTPChannel::~nsHTTPChannel   
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHTTPChannel.cpp
nsHTTPChannel::Release  
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHTTPChannel.cpp
nsXPCWrappedNative::~nsXPCWrappedNative 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp
nsXPCWrappedNative::Release     
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp
nsXPCWrappedNative::JSObjectFinalized   
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp
WrappedNative_Finalize  
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp
js_FinalizeObject       [d:\builds\seamonkey\mozilla\js\src\jsobj.c
js_GC   [d:\builds\seamonkey\mozilla\js\src\jsgc.c
js_ForceGC      [d:\builds\seamonkey\mozilla\js\src\jsgc.c
js_DestroyContext       [d:\builds\seamonkey\mozilla\js\src\jscntxt.c
JS_DestroyContext       [d:\builds\seamonkey\mozilla\js\src\jsapi.c
mozJSComponentLoader::~mozJSComponentLoader     
[d:\builds\seamonkey\mozilla\js\src\xpconnect\loader\mozJSComponentLoader.cpp
mozJSComponentLoader::`scalar deleting destructor'
mozJSComponentLoader::Release   
[d:\builds\seamonkey\mozilla\js\src\xpconnect\loader\mozJSComponentLoader.cpp
nsSupportsHashtable::ReleaseElement     
[d:\builds\seamonkey\mozilla\xpcom\ds\nsHashtable.cpp
_hashEnumerate  [d:\builds\seamonkey\mozilla\xpcom\ds\nsHashtable.cpp
PL_HashTableEnumerateEntries    [plhash.c
nsHashtable::Enumerate  [d:\builds\seamonkey\mozilla\xpcom\ds\nsHashtable.cpp
nsSupportsHashtable::~nsSupportsHashtable       
[d:\builds\seamonkey\mozilla\xpcom\ds\nsHashtable.cpp
nsSupportsHashtable::`vector deleting destructor'
NS_ShutdownXPCOM        [d:\builds\seamonkey\mozilla\xpcom\build\nsXPComInit.cpp
NETSCP6.EXE + 0x123b (0x0040123b)
NETSCP6.EXE + 0x27c7 (0x004027c7)
KERNEL32.DLL + 0x1b560 (0xbff8b560)
KERNEL32.DLL + 0x1b412 (0xbff8b412)
KERNEL32.DLL + 0x19dd5 (0xbff89dd5)
0xc4fe4300
0x800093f7

    nsCacheEntryChannel::~nsCacheEntryChannel 1d2e940e
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 26  Total: 30 
        OS: Windows NT  4.0 build 1381
        URL: http://www.sony.com
        Comment: tried to select a item in the combo box
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16182920

    nsCacheEntryChannel::~nsCacheEntryChannel d1564017
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 29  Total: 29 
        OS: Windows NT  4.0 build 1381
        URL: 
        Comment: exiting a mail message
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16187156

    nsCacheEntryChannel::~nsCacheEntryChannel 84ebf1c5
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 28  Total: 28 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16187640

    nsCacheEntryChannel::~nsCacheEntryChannel 0bf77841
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-21 UptimeMinutes: 14  Total: 157 
        OS: Windows 98  4.10 build 67766446
        URL: realplayer.com
        Comment: shutting down managing bookmarks
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16193773

    nsCacheEntryChannel::~nsCacheEntryChannel a4f1ad81
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 328  Total: 328 
        OS: Windows 98  4.10 build 67766446
        URL: editing imported IE bookmarks
        Comment: i was trying to delete some bookmarks that i didn't not need 
when netscape just froze up when i tried to exit it crashed necko.dll
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16193855

    nsCacheEntryChannel::~nsCacheEntryChannel 16312f87
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 17  Total: 30 
        OS: Windows 95  4.0 build 67306684
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16195318

    nsCacheEntryChannel::~nsCacheEntryChannel d109ae1b
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 299  Total: 1015 
        OS: Windows 98  4.10 build 67766446
        URL: realaudio download
        Comment: editing bookmarks
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16196842

    nsCacheEntryChannel::~nsCacheEntryChannel d109ae1b
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-22 UptimeMinutes: 19  Total: 19 
        OS: Windows 98  4.10 build 67766446
        URL: ask jeevs
        Comment: asking a question about china
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16201781

    nsCacheEntryChannel::~nsCacheEntryChannel e2a2e559
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 28  Total: 216 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: I was just closing down netscape.
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16203125

    nsCacheEntryChannel::~nsCacheEntryChannel 8d7339af
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 28  Total: 133 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16212205

    nsCacheEntryChannel::~nsCacheEntryChannel d109ae1b
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-21 UptimeMinutes: 20  Total: 185 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16218878

    nsCacheEntryChannel::~nsCacheEntryChannel 299f9bd2
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 10  Total: 261 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: it keeps crashing & though it doesn't freeze the whole computer
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16220649

    nsCacheEntryChannel::~nsCacheEntryChannel cacd908c
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 172  Total: 509 
        OS: Windows 98  4.10 build 67766446
        URL: www.webpersonals.com
        Comment: logging off webpersonals and netscape
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16224410

    nsCacheEntryChannel::~nsCacheEntryChannel 9e612af0
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 13  Total: 13 
        OS: Windows 95  4.0 build 67109975
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16225340

    nsCacheEntryChannel::~nsCacheEntryChannel 4f6eab8e
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 179  Total: 1687 
        OS: Windows 98  4.10 build 67766446
        URL: http://www.slashdot.org/interviews/00/07/20/1440204.shtml
        Comment: closing application.
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16226284

    nsCacheEntryChannel::~nsCacheEntryChannel 068a57ee
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 14  Total: 54 
        OS: Windows 98  4.10 build 67766222
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16226960

    nsCacheEntryChannel::~nsCacheEntryChannel 8d7339af
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 24  Total: 24 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16229011

    nsCacheEntryChannel::~nsCacheEntryChannel 1d2e940e
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 59  Total: 59 
        OS: Windows NT  4.0 build 1381
        URL: 
        Comment: Exiting.
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16233186

    nsCacheEntryChannel::~nsCacheEntryChannel 60911531
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 2849  Total: 4365 
        OS: Windows NT  4.0 build 1381
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16240648

    nsCacheEntryChannel::~nsCacheEntryChannel 77161501
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 179  Total: 1833 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: I hit quit
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16250965

    nsCacheEntryChannel::~nsCacheEntryChannel b43ccfe6
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 91  Total: 1211 
        OS: Windows 98  4.10 build 67766222
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16253006

    nsCacheEntryChannel::~nsCacheEntryChannel 4f6eab8e
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 253  Total: 381 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: I was exiting Netscape
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16257751

    nsCacheEntryChannel::~nsCacheEntryChannel 0b6590bf
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 111  Total: 492 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: exiting netscape
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16262008

    nsCacheEntryChannel::~nsCacheEntryChannel 351a46be
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-24 UptimeMinutes: 4  Total: 58 
        OS: Windows 98  4.10 build 67766222
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16264183

    nsCacheEntryChannel::~nsCacheEntryChannel d109ae1b
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 52  Total: 268 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: 
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16264614

    nsCacheEntryChannel::~nsCacheEntryChannel 664300a4
        
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/mgr/nsCacheEnt
ryChannel.cpp line 55
        Build: 2000080712 CrashDate: 2000-08-23 UptimeMinutes: 63  Total: 63 
        OS: Windows 98  4.10 build 67766446
        URL: 
        Comment: I was exiting Netscape6 when it failed
        Stacktrace: 
http://cyclone/reports/stackcommentemail.cfm?dynamicBBID=16272941

Brendan Eich [:brendan]

Reporter

Comment 9

•

24 years ago

This one is crying jband's name like a lost kitten mewing in the night.  How
could anyone refuse? ;-)

/be

Assignee: brendan → jband

Status: ASSIGNED → NEW

Brendan Eich [:brendan]

Reporter

Comment 10

•

24 years ago

Attached patch is this comment in the right place, or heading in the right direction? — Details — Splinter Review

John Bandhauer

Comment 11

•

24 years ago

I looked that stacks that chofmann posted. *every* one of them is a shutdown 
crash with NS_ShutdownXPCOM and mozJSComponentLoader::~mozJSComponentLoader on 
the stack (even though some of the comments claim that the user didn't think 
they were exiting the app).

I'm quite certain that this crash path was fixed in bug 49748.

I'm not sure we have a random crash bug here at all or that 
JS_{Begin,End,Suspend,Resume}Request is required. Data in bug 50859 suggest that 
we aren't even calling xpconnect on any other threads.

John Bandhauer

Updated

•

24 years ago

Status: NEW → ASSIGNED

John Bandhauer

Comment 12

•

24 years ago

I have a patch to use JS_{Begin,End}Request in xpconnect. 

I'll attach it. It includes changes for bug 52579 too.

I *think* these are the only places in xpconnect where these calls are 
necessary.

I don't see any reason to mess with JS_SetContextThread since we never use any 
JSContext on more than one thread.

BTW, It seems like JS_RestoreContextThread(JSContext*, intN) would be useful if 
a secondary thread were going to use a given context. Adding this call would 
allow the secondary thread to temporarily use the JSContext and then restore its 
cx->thread without requiring the primary thread to actively do anything special.
Of course, that second thread would have to ensure that the primary thread did 
not enter JS using the JCContext in question while the secondary thread was 
using it.

John Bandhauer

Comment 13

•

24 years ago

Attached patch proposed fix — Details — Splinter Review

Brendan Eich [:brendan]

Reporter

Comment 14

•

24 years ago

Looks good, but I think we do need JS_SetContextThread to be called for all cx's
used on non-main threads.  See js_GC(), near line 976 where if (cx->thread) is
tested.  If a cx has a zero thread-id, it's assumed to be not part of the
request model, and in fact if it's in a request an assertion will botch.

This special zero-thread-id case is to support the potentially large number of
contexts in the main (UI/DOM/layout) thread, which is where GC must run, and
where JS evaluations and other API call batches do not run within requests.

We don't pool contexts, or otherwise share a cx serially among two or more
threads, so no rush on JS_RestoreContextThread.  Can you arrange to
JS_SetContextThread at most once per cx/non-main-thread, by testing first with
JS_GetContextThread?

/be

John Bandhauer

Comment 15

•

24 years ago

Doesn't the call to js_InitContextForLocking in js_NewContext do all that needs 
to be done (given that no contexts are used on multipe threads)?

Brendan Eich [:brendan]

Reporter

Comment 16

•

24 years ago

Devious bajoran code.  You're right.

This means that js_GC is searching the context list for each GC.  Probably not a
huge deal, but it bugs me.  Maybe DOM contexts should do JS_SetContextThread(cx,
0)?

/be

Brendan Eich [:brendan]

Reporter

Comment 17

•

24 years ago

Does this request stuff happen even if the XPConnect is running on the main
thread?  It need not, and would save some overhead.  But you'd have to test what
thread is calling JS, and compare to a known-to-be-main thread (xpcom provides a
way to get the main thread as an nsIThread*, IIRC).

/be

John Bandhauer

Comment 18

•

24 years ago

You meant JS_ClearContextThread. There is no JS_SetContextThread(cx, 0)

Yes, I *could* make the Begin/End calls only if not on the main thread. 
xpconnect can cache the main thread (gotten from xpcom as suggested). It would 
cost a getthreadid call each time - cheaper than what happens in 
Begin/EndRequest.

Shall we do that? We can call JS_ClearContextThread when creating DOM contexts, 
but what about others created on the main thread? It would be high handed for 
xpconnect to be calling JS_ClearContextThread.

Another strategy for xpconnect would be to only do Begin/End calls if 
JS_GetContextThread returns non-0... no getthreadid call.

Setting cx->thread = 0 for DOM contexts is then a completely separate issue.

John Bandhauer

Comment 19

•

24 years ago

Attached patch proposed fix based on current tree — Details — Splinter Review

Brendan Eich [:brendan]

Reporter

Comment 20

•

24 years ago

I like your last idea, it's coherent with the GC.  If JS_GetContextThread
returns 0, don't do the request stuff.  Go for it, I'll work with jst on the DOM
JS_ClearContextThread calls.

/be

Brendan Eich [:brendan]

Reporter

Comment 21

•

24 years ago

a=brendan on the cumulative patch.  Go fast!

/be

John Bandhauer

Comment 22

•

24 years ago

I checked in the xpconnect fix.

We should do something in the JS Component loader too.

John Bandhauer

Comment 23

•

24 years ago

I've fixed this for xpconnect and just now for the JS component loader (checked 
in patch in bug 52936). Do we have any other places where this needs to be 
fixed?

John Bandhauer

Comment 24

•

24 years ago

libpref has one and only one JS_{Begin,End}Request pair. This is in the method: 
useLockPrefFile.
http://lxr.mozilla.org/seamonkey/search?string=useLockPrefFile
This method is not called by any code in Seamonkey. The one place that looks 
like a call is commented out.

John Bandhauer

Comment 25

•

24 years ago

cleaning up whiteboard and keywords - not dogfood

Keywords: dogfood, nsbeta2

Whiteboard: [dogfood-] [nsbeta2-]

Péter Bajusz

Updated

•

24 years ago

Target Milestone: M18 → ---

John Bandhauer

Comment 26

•

23 years ago

AFAICT prefs is the only code broken in this way.

Component: XPConnect → Preferences: Backend

Summary: JS on threads other than "main" must run within requests → [prefs]JS on threads other than "main" must run within requests

Alec Flett

Comment 27

•

23 years ago

broken in what way? prefs was never intended to run on any thread other than
"main" and if anyone is calling it from another thread, then they should be shot.

John Bandhauer

Comment 28

•

23 years ago

Whether or not prefs are called on other threads is unknown. We expose the 
subsystem to plugins. Should we decide that this just does not need such 
protection and close the bug?

Alec Flett

Comment 29

•

23 years ago

I thought plugins ran on the main thread as well..besides, do we expose the
entire prefs interface to plugins, or do we do it via the capability API. If
it's the capability API, then we need to make sure they proxy appropriately
basically we should be proxying calls to prefs if we're not on the main thread.

Brendan Eich [:brendan]

Reporter

Comment 30

•

23 years ago

jband: the DOM code that enters the JS engine does not use requests, right?  But
it will once your branch lands, eh?

alecf: we are just asking for it.  Some plugins are multi-threaded, and murphy
was an optimist.

/be

John Bandhauer

Comment 31

•

23 years ago

brendan: Yes. With XPConnect changes, native code calling into JS will use a 
request IFF JS_GetContextThread returns non-null. AFAICT we don't have any 
JSContexts that actually clear their context thread (it is set in JS_NewContext)
http://lxr.mozilla.org/seamonkey/search?string=ContextThread

A change I am making is to enter that request early on the way into xpconnect 
from native rather than wait until actually calling JS. This allows me to rely 
on protection from gc calls in xpconnect.

Mike Shaver (:shaver -- probably not reading bugmail closely)

Assignee

Comment 32

•

23 years ago

If prefs isn't intended to be running on non-main threads, let's see some
assertions in there!

What's the verdict here?  I can trace all the JSAPI usage in prefs and sprinkle
magic JS_*Request fairy dust, or I can declare victory and not do so.  Just tell
me what the plan should be here.

Assignee: jband → shaver

Status: ASSIGNED → NEW

Target Milestone: --- → mozilla0.9

John Bandhauer

Comment 33

•

23 years ago

shaver: I think this would be a good thing to assure (one way or the other). I
don't see anything wrong with bracketing the calls with requests.

But if you are going to touch prefs at all, I'd say please handle bug 71237 first.

Thanks,

John.

Mike Shaver (:shaver -- probably not reading bugmail closely)

Assignee

Comment 34

•

23 years ago

Patch for this is in 71237:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=28082

Status: NEW → ASSIGNED

Mike Shaver (:shaver -- probably not reading bugmail closely)

Assignee

Updated

•

23 years ago

Status: ASSIGNED → RESOLVED

Closed: 23 years ago

Resolution: --- → FIXED

Mike Shaver (:shaver -- probably not reading bugmail closely)

Assignee

Comment 35

•

23 years ago

In the tip.

is this comment in the right place, or heading in the right direction? 24 years ago Brendan Eich [:brendan] 796 bytes, patch		Details \| Diff \| Splinter Review
proposed fix 24 years ago John Bandhauer 2.71 KB, patch		Details \| Diff \| Splinter Review
proposed fix based on current tree 24 years ago John Bandhauer 840 bytes, patch		Details \| Diff \| Splinter Review