443874 - Need a hook for thread creation and destruction in thread pool

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

The JS engine asserts that all contexts created on a particular thread have been destroyed by the time the thread dies, and it asserts this at thread destruction time via NSPR's per-thread data destruction mechanism. I'm using a thread pool to create per-thread contexts in bug 437152 and I needed a way to be notified when a thread was created and destroyed in the pool to prevent the assertions. I couldn't use the per-thread data destructor mechanism because the JS engine does the same thing (and registers its TLS index first).

Attached is one possible fix. Benjamin, what do you think?

Comment 1

[Benjamin Smedberg]

Comment on attachment 328298 [details] [diff] [review]
Patch, v1

I'd really love a test for this...

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v2

Ok, this has tests. It also makes the thread pool release the listener after the shutdown() method rather than waiting until the thread pool is destroyed.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed to moz-central.

Mossop realized I forgot to bump the uuid of nsIThreadPool. Checked in a followup that did only that.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

The test is hanging on one windows box... I disabled it for the moment until I get a windows build going.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1216978929.1216982107.8538.gz&fulltext=1#err0
MacOSX Darwin 9.2.2 mozilla-central qm-moz2mini01

Finished running TestTextFormatter
Running TestThreadPoolListener
Running ThreadPoolListener tests...
../../dist/bin/run-mozilla.sh: line 131: 90077 Segmentation fault      "$prog" ${1+"$@"}
make[2]: *** [check] Error 139
make[1]: *** [check] Error 2
make: *** [check] Error 2

Comment 6

[Robert Kaiser]

SeaMonkey Mac (Tiger) is timing out on that test frequently, it only passes very rarely.

Comment 7

[Dave Townsend [:mossop]]

I've backed this out with changeset 29af572a67ba due to multiple test failures.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I believe that the real fix for the test failures is bug 449822, but I'll test by a cheap workaround.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Fix test

This applies on top of attachment 330118 [details] [diff] [review] and, if my theory is correct, should fix the hanging/crashing unit tests we saw on tinderbox.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Checked in, and it stuck this time!