Remove POSIX fd-backed SharedMemoryHandle on Mac |
|||||
Issue descriptionOn Mac, there are currently two implementations for SharedMemoryHandle: the performant and commonly used Mach VM one, and the slow and legacy POSIX fd-backed one [1]. Ideally we'd like to remove the latter and only use Mach, which we did at one point. But the POSIX implementation was resurrected for a single client [2][3]. This choice should be revisited and Finch should use Mach shared memory to receive the Finch data, so that we can re-remove the POSIX SharedMemoryHandle implementation on Mac. [1] https://chromium.googlesource.com/chromium/src/+/2bdaf0af5e6ab06707931b0c1f8a79635219c024/base/memory/shared_memory_handle_mac.cc [2] https://codereview.chromium.org/2555483002/ [3] https://chromium.googlesource.com/chromium/src/+/2bdaf0af5e6ab06707931b0c1f8a79635219c024/base/metrics/field_trial.cc#1289
,
Oct 19
,
Oct 19
,
Oct 22
I also planned to work on this issue since we're not going to add the POSIX fd-backed implementation to the new shared memory API on Mac. The client in question just should be switched to the new API. Let me know if there is anything I can help you with.
,
Oct 22
Thanks. It's a little complex because the field trial shmem region is needed before Mojo is initialized. I've got a WIP CL to do something that's Mac-specific: https://chromium-review.googlesource.com/c/chromium/src/+/1292211 I wasn't planning to transition the FieldTrialList to the new shared memory API, but just getting it off (and then removing) the FD-based one.
,
Oct 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4db5285bf54a8b2d77f8674e6775c4812899d09 commit e4db5285bf54a8b2d77f8674e6775c4812899d09 Author: Robert Sesek <rsesek@chromium.org> Date: Sat Oct 27 01:02:20 2018 [Mac] Replace FieldTrialList's FD-based shared memory with Mach shared memory. Field trials are initialized in child processes before the IPC channel is set up, so the Mach memory object cannot be transferred using it. And there is no way to transfer Mach ports across child creation, unlike with FDs. This CL introduces the FieldTrialMemoryServer, which is a Mach message server that publishes its endpoint in the bootstrap namespace. Child processes can look up the server and send it a message to request the field trial shared memory object. Access is limited to direct children of the server process. FieldTrialList is the sole caller of the legacy FD-backed shared memory API on Mac, and it needs to be removed in order to transition to new shared memory API (https://crbug.com/795291). Bug: 776564 Change-Id: I0e25469d6b28533a1ab26a4c8907cc704bc3c0fd Reviewed-on: https://chromium-review.googlesource.com/c/1292211 Commit-Queue: Robert Sesek <rsesek@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org> Cr-Commit-Position: refs/heads/master@{#603272} [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/BUILD.gn [add] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/mac/scoped_mach_msg_destroy.h [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/OWNERS [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial.cc [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial.h [add] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial_memory_mac.cc [add] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial_memory_mac.h [add] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial_memory_mac_unittest.cc [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/base/metrics/field_trial_unittest.cc [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/content/browser/child_process_launcher_helper_posix.cc [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/content/child/field_trial.cc [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/services/service_manager/sandbox/mac/common.sb [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/services/service_manager/sandbox/mac/common_v2.sb [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/services/service_manager/sandbox/mac/sandbox_mac.h [modify] https://crrev.com/e4db5285bf54a8b2d77f8674e6775c4812899d09/services/service_manager/sandbox/mac/sandbox_mac.mm
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b0cfc3f36ff400b672967d408a71c349a56b3537 commit b0cfc3f36ff400b672967d408a71c349a56b3537 Author: Robert Sesek <rsesek@chromium.org> Date: Tue Oct 30 03:03:33 2018 [Mac] Remove FD-backed shared memory implementation. There are no more users of this implementation, and the new shared memory APIs do not support FD-backed shared memory on Mac. Bug: 776564 Change-Id: I9307a6e46daa4329f0d04e6951b6e432a79c2c47 Reviewed-on: https://chromium-review.googlesource.com/c/1303873 Reviewed-by: Alexandr Ilin <alexilin@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#603757} [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/platform_shared_memory_region_mac.cc [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/shared_memory.h [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/shared_memory_handle.h [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/shared_memory_handle_mac.cc [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/shared_memory_mac.cc [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/memory/shared_memory_unittest.cc [modify] https://crrev.com/b0cfc3f36ff400b672967d408a71c349a56b3537/base/test/test_shared_memory_util.cc
,
Oct 30
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by rsesek@chromium.org
, Oct 19