1332508 - Apply https://github.com/jemalloc/jemalloc/commit/d9f7b2a4307f7ff9f7a139b33d366d44e8a8b83d to mozjemalloc/replace-malloc

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Comment 1

[Mike Hommey [:glandium]]

More specifically, the part about zone_force_unlock. The refactor is not necessary.

Comment 2

[Mike Hommey [:glandium]]

Other relevant commit: https://github.com/jemalloc/jemalloc/commit/4e2e3dd9cf19ed5991938a708a8b50611aa5bbf8

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1332508 - Reinitialize allocator mutexes in fork() child processes.

Adapted from
https://github.com/jemalloc/jemalloc/commit/4e2e3dd9cf19ed5991938a708a8b50611aa5bbf8
and
https://github.com/jemalloc/jemalloc/commit/d9f7b2a4307f7ff9f7a139b33d366d44e8a8b83d

As per the latter commit, it would seem unlocking, in fork() child
processes, mutexes that were locked in the parent process is not really
well supported on OSX 10.12. The addition of the zone_reinit_lock
function in 10.12 supports this idea.

Review commit: https://reviewboard.mozilla.org/r/105932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105932/

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 8828606 [details]
Bug 1332508 - Reinitialize allocator mutexes in fork() child processes.

https://reviewboard.mozilla.org/r/105932/#review106896

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/516551993d16
Reinitialize allocator mutexes in fork() child processes. r=njn

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/516551993d16

Comment 7

[Kent James (:rkent)]

Not sure if we would apply it to TB esr45, but we should consider for 52 if mozilla does not take it.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8828606 [details]
Bug 1332508 - Reinitialize allocator mutexes in fork() child processes.

Approval Request Comment
[Feature/Bug causing the regression]: Low-level system library changes in OSX 10.12
[User impact if declined]: Theoretical chances of deadlocks when forking subprocesses (for e.g. e10s or plugins) on OSX 10.12. Not formally observed, though.
[Is this code covered by automated tests?]: Yes and no. That is, the code being changed is pretty much exercised by anything that will cause subprocesses to start, and if it caused problems, we'd have seen them already.
[Has the fix been verified in Nightly?]: That it doesn't break anything, yes. That it actually fixes the theoretical deadlocks has not been observed (the deadlocks haven't been observed in the wild in the first place ; at least I'm not aware of such reports).
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: this fix depends on the changes from bug 1286613. It /is/ possible to come up with a different patch to fix the same issue on beta if bug 1286613 is not approved.
[Is the change risky?]: No
[Why is the change risky/not risky?]: See "Is this code covered by automated tests?"
[String changes made/needed]: None

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 8828606 [details]
Bug 1332508 - Reinitialize allocator mutexes in fork() child processes.

Declining for beta as the dependency on bug 1286613 makes this quite intrusive.

Comment 10

[Jorg K (CEST = GMT+2)]

Uplifted to THUNDERBIRD520b2_2017020901_RELBRANCH branch:
https://hg.mozilla.org/releases/mozilla-beta/rev/5f22c17aeac2