1981471 - WeakCache<GCHashMap<>> can trigger assertions in StoreBuffer::checkAccess() when used with nursery pointers

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco) (PTO until 14th September)]

As discovered by Jan.

Access to the store buffer during sweeping is protected by a lock and we assert that it is held. Unfortunately we are failing to take the lock sometimes.

This is because GCHashMap::traceWeakEntries doesn't match the version in GCHashSet. It should not initialise e in the for loop:

https://searchfox.org/mozilla-central/rev/169951beec18e0505611537cfdcf7f7d3a330404/js/public/GCHashTable.h#91
https://searchfox.org/mozilla-central/rev/169951beec18e0505611537cfdcf7f7d3a330404/js/public/GCHashTable.h#280

Comment 1

[Jon Coppeard (:jonco) (PTO until 14th September)]

Attachment: Bug 1981471 - Part 1: Fix GCHashMap::traceWeakEntries so it uses its argument |e| rather than creating an Enum r?jandem

This is a problem because destroying the Enum may require taking the store
buffer lock which we don't hold at this point. That's why these methods take an
Enum, so that we can control its lifetime like this.

This brings it into line with the GCHasSet version.

Comment 2

[Jon Coppeard (:jonco) (PTO until 14th September)]

Attachment: Bug 1981471 - Part 2: Refactor weak cache test code r?jandem

This removes a bunch of duplicated code with templates.

Comment 3

[Jon Coppeard (:jonco) (PTO until 14th September)]

Attachment: Bug 1981471 - Part 3: Add test for sweeping WeakCached<GCHashMap<>> containing nursery pointers r?jandem

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b2858520e673
https://hg.mozilla.org/integration/autoland/rev/a4ac153694bb
Part 1: Fix GCHashMap::traceWeakEntries so it uses its argument |e| rather than creating an Enum r=jandem
https://github.com/mozilla-firefox/firefox/commit/b94374f99bcd
https://hg.mozilla.org/integration/autoland/rev/8dfa41c62f75
Part 2: Refactor weak cache test code r=sfink
https://github.com/mozilla-firefox/firefox/commit/2f553bb44f47
https://hg.mozilla.org/integration/autoland/rev/653eb1da5405
Part 3: Add test for sweeping WeakCached<GCHashMap<>> containing nursery pointers r=jandem,sfink
https://github.com/mozilla-firefox/firefox/commit/773109e7a452
https://hg.mozilla.org/integration/autoland/rev/c1a6cbe444a7
apply code formatting via Lando

Comment 5

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/a4ac153694bb
https://hg.mozilla.org/mozilla-central/rev/8dfa41c62f75
https://hg.mozilla.org/mozilla-central/rev/653eb1da5405
https://hg.mozilla.org/mozilla-central/rev/c1a6cbe444a7