1409273 - Replace malloc_mutex_* with class methods

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Mike Hommey [:glandium]]

This is preparatory work to later use RAII for mutexes.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1409273 - Replace malloc_mutex_* with class methods.

Ideally, we'd be reusing some Mutex class we have in Gecko, the base one
in mozglue/misc being the best candidate. However, the contraints in
mozjemalloc make that unconvenient:
- Can't have a constructor because malloc_init() would likely run before
  it, and that would mean the mutexes would be re-initialized.
- Can't have a destructor because code will run after static
  destructors, and some of that code likely will invoke the allocator,
  and we can't have destructed mutexes by then.
- Can't use pthread_mutex on OSX because that loops back into the
  allocator.

Accomodating the use of Gecko mutexes around those constraints would
mean much more code than just implementing a new mutex class, so the
latter is preferred.

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

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 8919171 [details]
Bug 1409273 - Replace malloc_mutex_* with class methods.

https://reviewboard.mozilla.org/r/190082/#review195328

r=me with the issue below fixed.

::: memory/build/mozjemalloc.cpp:3900
(Diff revision 1)
>    unsigned i;
>    arena_bin_t* bin;
>    size_t prev_run_size;
>  
> -  if (malloc_mutex_init(&mLock))
> +  if (mLock.Init())
>      return true;

You inverted the sense of Mutex::Init()'s return value so you need to invert this condition. And then you could likewise invert the sense of arena_t::Init()'s return value (and its call sites!)

Comment 3

[Mike Hommey [:glandium]]

I'll leave arena_t::Init for a followup.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8919171 [details]
Bug 1409273 - Replace malloc_mutex_* with class methods.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190082/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d93b4fb4b17
Replace malloc_mutex_* with class methods. r=njn

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2d93b4fb4b17