x = [] try { (function() { schedulegc(1); ((function() { return { y: function() { u() = [] } } })()) })() watch.call(x, "valueOf", String) gc() } catch (e) {} try { (function() { x.valueOf = (function() { y() }) })() x + 2 print('foo') } catch (e) {} shows nothing on js debug shell (tested with a threadsafe deterministic 64-bit debug build) on m-c changeset b5d24ef1eb37 without any CLI arguments, but shows the following with --no-baseline --no-ion --no-ti: foo autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: http://hg.mozilla.org/mozilla-central/rev/7689530c9fc6 user: Jon Coppeard date: Wed Oct 02 10:38:40 2013 +0100 summary: Bug 913261 - GenerationalGC: Fix watchpoint rekeying r=terrence Jon, is bug 913261 a likely regressor? My configure options are: sh ./configure --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>

Yes, it looks like rekeying still isn't correct here.

I'm so sorry, Jon. All these custom hash tables, each with intricate custom GC code---from the outside, it strikes me as more than a little precarious, and I feel somewhat responsible, since cross-compartment wrapper tables: me jswatchpoint tables: me debugger cross-compartment referent tables: me oh yeah, Map and Set: me WeakMap: not me! I was only the reviewer on that one :-| Anyway. Say you find the bug and fix this one, at that point how confident should we be that they're all finally correct top to bottom? Is there another possible approach?

(In reply to Jason Orendorff [:jorendorff] from comment #2) Well I don't think it's the implementation, it's just the interaction of hashtables with moving GC is fundamentally really complicated :) It is worrying that this stuff is so hard to get right. I suspect that the other types are better exercised that the watchpoint map though. And the fuzzing is catching things which is good. Some ideas for improving our testing, we could: - write specific unit tests for post barriering maps and sets with HashKeyRef - write soak tests for watchpoints/maps/sets/etc As for an alternative design that would make this stuff easier, or at least abstract it to a smaller core, I can't think of anything right now that would help.

My change to the call to rekeyFront() in the fix for bug 913261 was wrong, as was the explanation for what was wrong :-/ The single argument version is just fine here, as we don't lookup the entry at all. Also, we don't need or want to post-barrier the watchpoint map as we already mark and update all its keys in markAll(), which is called when we mark the runtime. So all this code can go. I checked that this fixes both problems.

Comment on attachment 814976 [details] [diff] [review] bug924690-watchpoint-fuzz Review of attachment 814976 [details] [diff] [review]: ----------------------------------------------------------------- I could have sworn that there was some case where markAll() was not adequate. Oh well, our fuzzers will find it soon, if it's still relevant. r=me