With async video enabled, we sometimes allocate shared memory from the video decoder thread.

Don't we need to change SharedMemory::Create and company to atomically modify the counter? (Or we can make the counter an atomic from mfbt/Atomics.h.) Or did you do this already?

You're right, we definitely do need to do this. I just blindly assumed the macro handled this, which doesn't make any sense.

Oh, you also need to change the constructor, unless you've done that already. There's a CAS operator you can use in Atomics.h, if you want.

> unless you've done that already. I mean, in one of the related bugs.

Which constructor do I need to change? The gShmemAllocated/gShmemMapped values should be statically initialized to 0. mAllocSize/mMappedSize are per-instance and don't need to be accessed from multiple threads.

> static bool registered; > if (!registered) { This isn't threadsafe; it needs to do a compare-and-swap.

So, Atomic<int64_t> doesn't work on our 32 bit platforms (which is most of them). I think switching to a 32bit type on 32bit platforms should be fine here, thoughts?

> I think switching to a 32bit type on 32bit platforms should be fine here, thoughts? If we allocate more than 2^31 bytes of memory on a 32-bit platform we're in deep trouble, so this seems fine to me.

Comment on attachment 755808 [details] [diff] [review] Make SharedMemory's reporter threadsafe v3 r=me. The assertions are now not guaranteed to find all problems (because it's possible for two Unmapped() calls to run at the same time), but that's fine with me; you won't get a false-positive. If you wanted to be guaranteed to catch the values going negative, you could switch to ssize_t, at the cost of one bit of space. Or I think you can call -= such that it returns the new or old value of the variable, and you could use that to check for overflow after the fact.