1950041 - Race condition between deferred purge and deleting a private arena

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Paul Bone [:pbone]]

This doesn't happen in practice because nothing in Firefox deletes an arena off-main-thread. But fixing this will make it safer to use arenas.

Thread 1: running MayPurgeStep, finds arena in the list, releases list lock.
Thread 2: Destroys arena.
Thread 1: Attempts to purge destroyed arena.

There's a couple of other related cases such as when a Purge() is already underway but releases the lock for the system call. this wasn't previously a problem because although it released the lock it was synchronous with an explicit call to free() or realloc() or the arena collection lock was held for the duration of the purge.

Comment 1

[Paul Bone [:pbone]]

Attachment: Bug 1950041 - Fix a comment r=glandium

Comment 2

[Paul Bone [:pbone]]

Attachment: Bug 1950041 - Use an enum for Purge()'s return value r=glandium

Comment 3

[Paul Bone [:pbone]]

Attachment: Bug 1950041 - Delay destruction of an arena until purging is complete r=glandium

Comment 4

[Paul Bone [:pbone]]

Beta is unaffected because no code currently deletes arenas off-main-thread and all delayed purge occurs on the main thread.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1903758

Comment 6

[Jens Stutte [:jstutte]]

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #5)

Set release status flags based on info from the regressing bug 1903758

Note that we currently do not delete arenas off main thread, so there is no release impact.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1903758

Comment 8

[Paul Bone [:pbone]]

Attachment: Bug 1950041 - Use an enum for the kind of purge condition r=glandium

Comment 9

[Paul Bone [:pbone]]

Note that this only applies to private arenas which were safe for deletion before Bug 1903758. Thread-local (aka public) arenas aren't safe for deletion because they're used implicitly (Bug 1364359).

Comment 10

[Dianna Smith [:diannaS]]

Hi :pbone! were you going to land this on 138? Since this doesn't affect beta, wondering if you should land soon for it to get good bake time in nightly.

Comment 11

[Paul Bone [:pbone]]

Hi Dianna, Yes this should land soon. Bug 1947687 is landing now which this depends on.

Nightly is also currently unaffected like beta, It's a theoretical thing that we're not making use of yet in Firefox.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70ec52f9316c
Fix a comment r=glandium
https://hg.mozilla.org/integration/autoland/rev/6771560ca736
Use an enum for the kind of purge condition r=glandium
https://hg.mozilla.org/integration/autoland/rev/2efb94b34159
Use an enum for Purge()'s return value r=glandium
https://hg.mozilla.org/integration/autoland/rev/267514795e90
Delay destruction of an arena until purging is complete r=glandium,jstutte

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/70ec52f9316c
https://hg.mozilla.org/mozilla-central/rev/6771560ca736
https://hg.mozilla.org/mozilla-central/rev/2efb94b34159
https://hg.mozilla.org/mozilla-central/rev/267514795e90