1465667 - Mac SharedMemoryBasic has an unintentional 128MiB size limit

Status: RESOLVED FIXED

(Core :: IPC, enhancement, P1)

Description

[Jed Davis [:jld] (away until 10-15)]

The way we're currently implementing shared memory on Mac OS X, if I understand the Mach APIs correctly, is that we first allocate and map a virtual memory area using mach_vm_allocate (similar to mmap with MAP_ANON) and then obtain a shareable capability for the underlying VM object using mach_make_memory_entry_64.

However, the memory mapping is fragmented into multiple objects if it's over a certain size; bug 1458246 comment #5 reports a 256MiB limit, but when I tested with a small C program on 10.10 I observed a fragment size of 128MiB.  In the kernel source code this is ANON_CHUNK_SIZE[1][2][3]; according to the comment at [1] it's a workaround for a performance issue that doesn't seem to be relevant for our usage.  The result is that we get a Mach port that represents only the first 128 MiB of the memory.

The WebKit code cited in bug 1161166 comment #0 still exists[4] and still does the same thing we're doing.  (It also postdates ANON_CHUNK_SIZE, if I'm reading the commit logs right.)  Maybe they never use it for large allocations?

But there's an alternative: calling mach_make_memory_entry_64 with the flag MAP_MEM_NAMED_CREATE, if I understand correctly, will create a new memory object and return a port for it.  In my testing on 10.10, this works for sizes up to 4GiB - 4KiB (i.e., 0xFFFFF000 bytes, or ANON_MAX_SIZE[5]), and either returns an object of the requested size or fails if the size limit is exceeded.  Reading through the source seems to confirm that this is the current behavior.

Also, MAP_MEM_NAMED_CREATE is used internally by the kernel to implement both the POSIX[6] and SysV[7] shared memory APIs, although they store a sequence of Mach ports (so they can exceed ANON_MAX_SIZE) and would in theory tolerate getting a smaller VM object than requested.  (This was changed from the vm_allocate approach in xnu 1456.1.26, which seems to correspond to OS X 10.6.)

So, there's probably a simple fix by using MAP_MEM_NAMED_CREATE, because I don't think we'd ever need 4 GiB in a single shared memory segment.  If for some reason that doesn't work, then we'd need to either redesign our code to handle multiple ports, or else consider replacing it with shm_open and POSIX fd passing (see also bug 1447867).


[1] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/osfmk/mach/vm_param.h#L247
[2] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/osfmk/vm/vm_map.c#L2671
[3] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/osfmk/vm/vm_map.c#L2830
[4] https://github.com/WebKit/webkit/blob/d4d4383e08e71301b1e4fea3c046a1b5d924e82f/Source/WebKit/Platform/cocoa/SharedMemoryCocoa.cpp
[5] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/osfmk/mach/vm_param.h#L240
[6] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/posix_shm.c#L742
[7] https://github.com/opensource-apple/xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/sysv_shm.c#L797

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1465667 - Call mach_make_memory_entry_64 with MAP_MEM_NAMED_CREATE to exceed the 128mb limit on macOS

We were first allocating and mapping a virtual memory area using mach_vm_allocate
(similar to mmap with MAP_ANON) and then obtaining a shareable capability for the
underlying VM object using mach_make_memory_entry_64. However the memory mapping
is fragmented into multiple objects if it's over 128mb. Larger memory allocations
than 128mb weren't possible. To fix this, we are calling  mach_make_memory_entry_64
with MAP_MEM_NAMED_CREATE. That will create a new memory object and return a port
for it.

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

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

It looks like the patch works on my machine but also triggered a try build just in case. Thanks for the help Jed!

Comment 3

[Jed Davis [:jld] (away until 10-15)]

Comment on attachment 8982616 [details]
Bug 1465667 - Call mach_make_memory_entry_64 with MAP_MEM_NAMED_CREATE to exceed the 128mb limit on macOS

https://reviewboard.mozilla.org/r/248574/#review255324

Thanks for writing the patch.  r+ with one important change:

::: ipc/glue/SharedMemoryBasic_mach.mm:549
(Diff revision 1)
> -                                 MACH_PORT_NULL);
> +                                               MACH_PORT_NULL);
>    if (kr != KERN_SUCCESS || memoryObjectSize < round_page(size)) {
>      LOG_ERROR("Failed to make memory entry (%zu bytes). %s (%x)\n",
>                size, mach_error_string(kr), kr);
>      CloseHandle();
> -    mach_vm_deallocate(mach_task_self(), address, round_page(size));
> +    mach_vm_deallocate(mach_task_self(), 0, round_page(size));

This line should just be removed; this method is no longer creating a memory mapping that needs to be unmapped.  Trying to unmap at address 0 (nullptr) is definitely not what we want.

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Comment on attachment 8982616 [details]
Bug 1465667 - Call mach_make_memory_entry_64 with MAP_MEM_NAMED_CREATE to exceed the 128mb limit on macOS

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

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Thanks for the review! Removed `mach_vm_deallocate` line.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b8a579ca9dd2
Call mach_make_memory_entry_64 with MAP_MEM_NAMED_CREATE to exceed the 128mb limit on macOS r=jld

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/b8a579ca9dd2