SEGV during JS_GC in Multi-threaded Application

[Paul Barnetta]

Hi there,

I have a threading-related question I'm hoping someone will be able to
help me out with.

I'm using SpiderMonkey 1.7 and am experiencing random crashes in an
application I'm writing; running it under gdb I see the offending line
appears to be in js_GC:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb7bb9b90 (LWP 17800)]
0xb7e53e9d in js_GC (cx=0x8081960, gckind=GC_NORMAL) at jsgc.c:2853
2853 memset(acx->thread->gcFreeLists, 0, sizeof
acx->thread->gcFreeLists);
Current language: auto; currently c
(gdb) p acx
$1 = (JSContext *) 0x80820a0
(gdb) p acx->thread
$2 = (JSThread *) 0x0

In my application I maintain a pool of JSContexts: as they're requested
from the pool JS_SetContextThread and JS_BeginRequest are called; when
they're returned JS_EndRequest and JS_ClearContextThread are called. So
it seems an unused/returned context (for which JS_ClearContextThread has
been called so cx->thread == NULL) appears to be deferenced..

This is curious though because immediately before the memset a check is
done to ensure acx->thread is *not* NULL:

while ((acx = js_ContextIterator(rt, JS_FALSE, &iter)) != NULL) {
if (!acx->thread || acx->thread == cx->thread)
continue;
memset(acx->thread->gcFreeLists, 0, sizeof
acx->thread->gcFreeLists);
GSN_CACHE_CLEAR(&acx->thread->gsnCache);
}

So what I believe is happening (after peppering the sources with
debugging statements) is that GC begins in one thread immediately after
another thread finishes JS_EndRequest but before JS_ClearContextThread
gets called (but which then happens during the GC); ie:

T1: JS_GC()
T1: [blocks on other active requests]
T2: JS_EndRequest()
T2: acx->thread remains != NULL
T1: [resumes]
T1: ...
T1: if (!acx->thread || ...
T1: continue; /* not invoked */
T2: JS_ClearContextThread()
T2: acx->thread == NULL
T1: memset(acx->thread->gcFreeLists, ...
T1: *crash*

Could this be the case? Does it seem reasonable? Or am I doing something
else obviously wrong that's causing an issue? I'd appreciate any
thoughts and would be happy to dig/post more if you point me in the
right direction :)

Many thanks,
Paul

[Wes Garland]

Paul:

I doubt your hypothesis is correct, because if it was, I think i would have
hit it when I was doing MT work with spidermonkey 1.7, although I suppose
it's possible, as I allocate contexts differently than you. What I do is
maintain a 1:1 context:thread relationship. This makes context management
easy (no pool code), and context creation is probably not much more
expensive than your pool + JS_SetContext overhead.

I see js_GC was called with GC_NORMAL, this implies that you also have a
watchdog thread which spawns GC occasionally? Are you making sure you have
suspend/resume request around I/O and other time-consuming non-jsapi C code
-- no jsapi code at all while the requests are suspended?

Are you building a debug build? Are you enabling GC zeal? (in 1.7 I think
it's a compiler switch, -DWAY_TOO_MUCH_GC).

What you're describing smells to me more like a memory corruption error,
although it's hard to tell. If you run zealous GC you probably find that
your ka-boom will move a lot closer to the root cause of the problem.

Wes

--
Wesley W. Garland
Director, Product Development
PageMail, Inc.
+1 613 542 2787 x 102

[Igor Bukanov]

2009/2/4 Paul Barnetta <paul.b...@smx.co.nz>:

> So what I believe is happening (after peppering the sources with debugging
> statements) is that GC begins in one thread immediately after another thread
> finishes JS_EndRequest but before JS_ClearContextThread gets called (but
> which then happens during the GC); ie:

This is a plausible scenario and indeed shows a bug in SM. Please file
this at https://bugzilla.mozilla.org/enter_bug.cgi?product=Core . As a
workaround I would suggest not to use the same JSContext accross
different threads removing the need to call JS_ClearContextThread.

Regards, Igor

[Paul Barnetta]

Wes,

> I doubt your hypothesis is correct, because if it was, I think i would
> have hit it when I was doing MT work with spidermonkey 1.7, although I
> suppose it's possible, as I allocate contexts differently than you.
> What I do is maintain a 1:1 context:thread relationship.

In this case I presume you would never have had to call
JS_ClearContextThread and this would explain why you never saw the
issue? Assuming there is an issue, of course..

Igor,

> This is a plausible scenario and indeed shows a bug in SM. Please file
> this at https://bugzilla.mozilla.org/enter_bug.cgi?product=Core

Thanks, I've done this and added some sample code which seems to
reproduce the problem for me -- so I'll keep an eye on that for now.

https://bugzilla.mozilla.org/show_bug.cgi?id=476934

Many thanks,
Paul

[Wes Garland]

Actually, you know Paul -- now that you mention it, I *do* recall use
JS_ClearContextThread() before going with my 1:1 thread model. Maybe I'm
totally off base and I _did_ bump into your problem. *hhmmm*

What happens if you begin a request before JS_NewObject, and end it after
the for loop? (It's okay, requests nest). You have should have a request
around every place a JS_ function could allocate ram. (I just consider all
JS_ functions as needing requests).

Looking at your bug -- without running any code -- I actually think you may
have found a bug. That looks like a good test case.

Which "latest version" are you using? Your best bet is to do a pull from the
hg repository.

Oh, wait! Why don't you have a request around JS_ClearContextThread()? You
really should.

[Paul Barnetta]

Hi Wes,

> Which "latest version" are you using? Your best bet is to do a pull from the
> hg repository.

I tried this, but had the same problem..

> Oh, wait! Why don't you have a request around JS_ClearContextThread()? You
> really should.

Yip, this is the direction the discussion in bugzilla is taking..

I didn't wrap the call because the API docs for JS_ClearContextThread
doesn't mention this (and in fact doesn't demonstrate it in its own code
example); and as was mentioned in the bug by others, it seems pretty
non-intuitive.

Still, I'm happy to be learning more, whichever direction this goes
(JS_ClearContextThread becomes callable outside of a request and blocks
on GC or the documentation is tidied up a little).. :)

Thanks for your feedback,

Paul