Add an option to use /tmp instead of /dev/shm |
|||||||||||
Issue descriptionWe should make it possible to use /tmp (or another directory) instead of /dev/shm when the latter isn't available (e.g., Amazon Lambda).
,
Aug 18 2017
There appears to be a rather large group of people manually compiling Chromium just to avoid this particular issue: https://medium.com/@marco.luethy/running-headless-chrome-on-aws-lambda-fa82ad33a9eb and https://github.com/adieuadieu/serverless-chrome This issue is also a duplicate of https://bugs.chromium.org/p/chromium/issues/detail?id=715363
,
Sep 1 2017
This is an issue on other infra as well: https://discuss.circleci.com/t/selenium-standalone-chrome-crashes-without-larger-shm-or-shm-volume-mounted/11612/20
,
Sep 19 2017
skyostil@ any way we can bump the priority on this one soon? We're getting a lot of reports from developers that are trying to use Puppeteer in Docker. Many platforms would benefit: Google Cloud, Amazon Lambda, others. I'm also starting to see it on a playground site we're working on (https://try-puppeteer.appspot.com/).
,
Sep 19 2017
Issue 764569 has a bunch of crash reports from folks trying to run Chrome in Docker. The bug has well-written repros and good debugging work, so I think it's fair to say the reporters are solid developers making reasonable use of the available documentation and resources.
,
Sep 19 2017
@pwnall Is 764569 private? I'm not a contributor and I can't view it.
,
Sep 19 2017
Agreed that this is something we want to resolve. Let me see if I can find an owner.
,
Sep 28 2017
,
Sep 28 2017
Awesome that this is moving forward :) I noticed this bug is assigned to headless, and just wanted to point out it occurs in a pseudo-headless environment as well, using regular Chromium + xvfb. Hopefully a fix can easily address both scenarios.
,
Oct 17 2017
the small feature takes long time, any updates of this?
,
Nov 5 2017
Would also love to get some kind of status update, even if "it's still on the backburner". Thanks for the hard work!
,
Nov 5 2017
We are aiming to have this done this quarter, so indeed in still on the backburner
,
Nov 14 2017
+thestig does it make sense to use /tmp as a fallback for /dev/shm if it doesn't exist? IMO, I think it does not make sense at all for chrome to use /dev/shm. The only reason it would need to is if it shares the files with an external app, otherwise we would just be able to pass the fd from shm_open() over a pipe. wdyt?
,
Nov 14 2017
I strace'd Chrome and the only files created in /dev/shm were unlink()ed immediately afterwards.
,
Nov 14 2017
Well I guess this is why we don't do shm_open() https://cs.chromium.org/chromium/src/base/memory/shared_memory_helper.cc?rcl=53ccc526cac98cfe2e16d8c3d2c99b386a155381&l=40 But we could still do it on Linux?
,
Nov 16 2017
Also https://bugs.chromium.org/p/chromium/issues/detail?id=522853#c25 suggested to use memfd_create on linux. Fixing this will really help for container/kubernetes use cases.
,
Nov 16 2017
+reveman for his thoughts. Are there other shared memory folks who can chime in?
,
Nov 16 2017
Using memfd_create when available should be easy and that would eliminate all these /dev/shm issues. However, memfd_create appeared in Linux 3.17 so it's not a solution for old kernels, which means we'd still have to maintain the code for using /dev/shm in chromium for now.
,
Nov 16 2017
How can we check if memfd_create is available? I don't see many references in the codebase: https://cs.chromium.org/search/?q=+memfd_create&type=cs
,
Nov 16 2017
We can just try to use it and fallback to /dev/shm if kernel returns ENOSYS error to indicate that syscall doesn't exits.
,
Nov 17 2017
I'm supportive of splitting the Linux shared memory code from Mac. For memfd_create, just try calling it and see if the kernel returns ENOSYS. Save the result in a static base::Optional<bool> so future calls can skip trying to use memfd_create if it is unavailable. Most desktop users should have memfd_create, unless they are on Debian 8 or Ubuntu 14.04 before 14.04.5. We still need the fallback for Android/Chromecast/ChromeOS/etc where there may be devices stuck on older kernels. The Q&A in CreateAnonymousSharedMemory() is always good for a laugh.
,
Nov 20 2017
Thanks for the insights! I'm working on a patch. Handling the ENOSYS error might be a bit annoying, but I guess we need to support older kernel versions. afaik Android uses ashmem_create_region to handle shared memory
,
Nov 24 2017
Looks like in swarm tests we always get ENOSYS for memfd_create, so the tests pass if the fallback works (see this patch's run: https://chromium-review.googlesource.com/c/chromium/src/+/781160/15) Locally, I can test that mefd_create works for browsertests and unittest, but essentially submitting this cl would mean that the feature is untested in swarming. Any ideas on how to deal with this, I wonder if there are FYI bots
,
Nov 24 2017
> essentially submitting this cl would mean that the feature is untested in swarming. The only non-green bot I see for that patch set is android_n5x_swarming_rel. All of the other linux and android bots are green (and most of them use swarming for tests too, even though it's not in the name), so I would say we have plenty of coverage already. In fact, it's better that there's a config with an old kernel so that the old codepath has coverage too.
,
Nov 24 2017
Sorry, I should've been more clear: That run contains a CHECK(!result) that should create a crash if memfd_create actually succeeded with no ENOSYS error https://chromium-review.googlesource.com/c/chromium/src/+/781160/15/base/memory/shared_memory_posix.cc#143 That crashes on my local machine (with the valid kernel version) but no swarm bot actually crashes because they don't support it, and the fallback works as intended.
,
Nov 24 2017
Ah, that's not very good coverage then :) I think you can set 'os':'Ubuntu-16.04' in swarming_dimensions to get a Xenial machine with the newer kernel. We've been meaning to set up a Xenial config for a while but haven't gotten to it yet. You're more than welcome to set one up. +dpranke for visibility
,
Nov 24 2017
Are you referring to running an isolated testing? I tried following instructions at: https://www.chromium.org/developers/testing/isolated-testing/for-swes but the bot fails without running: https://chromium-swarm.appspot.com/task?id=3a0522d2a485ae10&refresh=10&show_raw=1
,
Nov 24 2017
Looks like you picked up a bot with an arm cpu. You also want 'cpu':'x86-64'. I mean that we need to get a full FYI bot set up.
,
Nov 27 2017
FYI sounds great. I'm still trying to get a run just in case before submitting, it looks like the bot is stuck on PENDING for some reason: https://chromium-swarm.appspot.com/task?id=3a147739de0d9c10&refresh=10&show_raw=1
,
Nov 29 2017
I'll go ahead and submit the cl. I opened https://crbug.com/789331 to see if this can be tested in swarm
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0165cbc5f8bec601e0d22cea49e043821d947b55 commit 0165cbc5f8bec601e0d22cea49e043821d947b55 Author: David Vallet <dvallet@chromium.org> Date: Wed Nov 29 22:21:30 2017 Use memfd_create for anonymous shared memory files in linux, failback to temp file if not available Test coverage: base_unittests --gtest_filter=SharedMemoryTest.* Bug: 736452 Change-Id: Ic4ed3b40cc1c62ae9bfc9e2c7590e4248a8c4485 Reviewed-on: https://chromium-review.googlesource.com/781160 Commit-Queue: David Vallet <dvallet@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#520269} [modify] https://crrev.com/0165cbc5f8bec601e0d22cea49e043821d947b55/base/memory/shared_memory_helper.cc [modify] https://crrev.com/0165cbc5f8bec601e0d22cea49e043821d947b55/base/memory/shared_memory_helper.h [modify] https://crrev.com/0165cbc5f8bec601e0d22cea49e043821d947b55/base/memory/shared_memory_mac.cc [modify] https://crrev.com/0165cbc5f8bec601e0d22cea49e043821d947b55/base/memory/shared_memory_posix.cc [modify] https://crrev.com/0165cbc5f8bec601e0d22cea49e043821d947b55/base/memory/shared_memory_unittest.cc
,
Nov 30 2017
c#32: Opened bug 789768 to track adding the Xenail bot. Now that the memfd_create patch has landed, do we still care about falling back to /tmp if /dev/shm doesn't exist and the kernel doesn't support memfd_create?
,
Nov 30 2017
Looks like AWS lambda is using kernel 4.9 http://docs.aws.amazon.com/lambda/latest/dg/current-supported-versions.html So in theory the fallback should not be necessary for the main use case, and probably not necessary. If anyone wants to test if that's the case that'd be great.
,
Nov 30 2017
Any idea in which stable version this will end up?
,
Nov 30 2017
I can confirm that this change works on AWS Lambda without the need for a fallback. That's great! Thank you all for your effort.
,
Nov 30 2017
> Any idea in which stable version this will end up? Chrome 64.
,
Nov 30 2017
64.0.3281.0 and newer, to be specific.
,
Nov 30 2017
#37: Thanks for checking! I'm inclined then to mark this as fixed, let me know if you feel otherwise
,
Nov 30 2017
Issue 764569 has been merged into this issue.
,
Nov 30 2017
c#40 I agree, marking as fixed. If anyone still needs to run chrome without /dev/shm on a kernel < 3.17, please shout
,
Dec 1 2017
Just attempted to confirm that this solves the issue in Google App Engine (flexible) as well. It looks like the runtimes in GAE Docker have a kernel version of 3.16.0-4-amd64. Unfortunately, this patch doesn't actually fix the issue for that (common) environment :(. Still could really use an option to use an alternative directory than /dev/shm there. Also, this is a duplicate/related bug in case someone wants to merge it in: https://bugs.chromium.org/p/chromium/issues/detail?id=715363
,
Dec 1 2017
Reopening based on c#43
,
Dec 1 2017
,
Dec 1 2017
> 64.0.3281.0 and newer, to be specific. how to install it? i installed unstable version, but there is 64.0.3278.0 version
,
Dec 1 2017
re: comment 46 - for Google Chrome, wait for an upcoming release. We have an archive of Chromium continuous builds for testing here as well: https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/60e1e0e9381d9965ee316752eaf3a239a764b03f commit 60e1e0e9381d9965ee316752eaf3a239a764b03f Author: David 'Digit' Turner <digit@google.com> Date: Tue Dec 05 18:57:50 2017 Disable memfd base::SharedMemory implementation. Since there is no way to make read-only file descriptors from Linux memfd-based regions, the security guarantees of GetReadOnlyHandle() cannot be maintained with this implementation. Remove it to fall-back on traditional Posix shared regions instead. Bug: 792117 , 736452 Change-Id: Ie5eb41fc3c4dd02ebdbb77be8375363ba51f1b00 Reviewed-on: https://chromium-review.googlesource.com/809014 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: David Turner <digit@chromium.org> Cr-Commit-Position: refs/heads/master@{#521768} [modify] https://crrev.com/60e1e0e9381d9965ee316752eaf3a239a764b03f/base/memory/shared_memory_posix.cc [modify] https://crrev.com/60e1e0e9381d9965ee316752eaf3a239a764b03f/base/memory/shared_memory_unittest.cc
,
Dec 7 2017
c#42 > If anyone still needs to run chrome without /dev/shm on a kernel < 3.17, please shout *shouts* - I do. Is there a build somewhere?
,
Dec 7 2017
An update: The use of memfd_create cl (https://crrev.com/c/781160) was reverted due to security concerns, so this issue is not fixed yet. We might try to reland it but first SharedMemory code needs to be refractored so SEAL_WRITE can be applied after writing the data. I'm trying to submit https://crrev.com/c/810325 which enables a flag to always use a temp directory for shmem files. You are free to patch it and try it.
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44b2c1177059576c9a5c5f9f1eefa9be29d64676 commit 44b2c1177059576c9a5c5f9f1eefa9be29d64676 Author: Robert Sesek <rsesek@chromium.org> Date: Fri Dec 08 18:34:40 2017 Disable memfd base::SharedMemory implementation. Since there is no way to make read-only file descriptors from Linux memfd-based regions, the security guarantees of GetReadOnlyHandle() cannot be maintained with this implementation. Remove it to fall-back on traditional Posix shared regions instead. TBR=digit@google.com (cherry picked from commit 60e1e0e9381d9965ee316752eaf3a239a764b03f) Bug: 792117 , 736452 Change-Id: Ie5eb41fc3c4dd02ebdbb77be8375363ba51f1b00 Reviewed-on: https://chromium-review.googlesource.com/809014 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: David Turner <digit@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#521768} Reviewed-on: https://chromium-review.googlesource.com/817682 Cr-Commit-Position: refs/branch-heads/3282@{#100} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/44b2c1177059576c9a5c5f9f1eefa9be29d64676/base/memory/shared_memory_posix.cc [modify] https://crrev.com/44b2c1177059576c9a5c5f9f1eefa9be29d64676/base/memory/shared_memory_unittest.cc
,
Dec 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1824e5752148268c926f1109ed7e5ef1d937609a commit 1824e5752148268c926f1109ed7e5ef1d937609a Author: David Vallet <dvallet@chromium.org> Date: Thu Dec 14 00:46:08 2017 Add switch to always use temp dir in shared memory instances on Linux builds Modify GetShmemTempDir to check for a runtime switch to always use the temp dir Changes tests in shared_memory_unittest.cc to be parameterized and test also for when /dev/shm is disabled. Bug: 736452 , 792117 Change-Id: Id930ea525980e97c4ab70e37689301370776fc79 Reviewed-on: https://chromium-review.googlesource.com/810325 Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: David Vallet <dvallet@chromium.org> Cr-Commit-Position: refs/heads/master@{#523959} [modify] https://crrev.com/1824e5752148268c926f1109ed7e5ef1d937609a/base/base_switches.cc [modify] https://crrev.com/1824e5752148268c926f1109ed7e5ef1d937609a/base/base_switches.h [modify] https://crrev.com/1824e5752148268c926f1109ed7e5ef1d937609a/base/files/file_util_posix.cc [modify] https://crrev.com/1824e5752148268c926f1109ed7e5ef1d937609a/base/memory/shared_memory_unittest.cc
,
Dec 14 2017
Glad to see this merged! I just tried to verify the fix in a local Docker container but am having trouble getting the --disable-dev-shm-usage switch to take effect: Specifically, I downloaded a linux-x64 build from https://download-chromium.appspot.com/, Build Revision: 523961 (corresponds to https://chromium.googlesource.com/chromium/src/+/bb64b765223d40ce09b898dc2d2a6029a33fcc76 which should include this commit). I then loaded it into a Docker container and launched Chrome manually via "./chrome --no-sandbox --disable-dev-shm-usage" while watching the size of the 64MB /dev/shm partition. The partition's usage still increased in size when Chrome was launched, and I was able to browse around Twitter's website enough to get Chrome to run out of /dev/shm space and crash. Adding/removing the switch doesn't seem to have any effect. Am I using the cmdline switch incorrectly, or is there some other issue?
,
Dec 14 2017
Thanks for checking! This change should have had the same effect as the one reverted (https://crrev.com/c/781160) when running with the flag, so maybe it needs further investigation. It would help knowing what files are being created in /dev/shm specifically. What I see on my linux workstations is a "pulse-shm-*" file being created which I was assuming is created by pulseaudio, which should be disabled in containers anyway, I guess.
,
Dec 14 2017
Interestingly, when I do a "ls -al /dev/shm", I'm seeing nothing at all while Chrome is running, both with and without the flag. However, running "watch -n 0.1 df -h" shows the partition filling up as Chrome is launched and sites are loaded. I'm not familiar enough with the details of shared memory allocation in Linux to know what's causing the discrepancy, but FWIW I'm seeing the same behavior on Chrome stable as well. Regarding the reverted change, I didn't notice it filling up /dev/shm at all when I tested it, and also confirmed that browsing behavior that reliably crashed Chrome (scrolling up and down on the Twitter signed-out homepage) did not crash Chrome. (As long as kernel >= 3.17 was used, of course.)
,
Dec 14 2017
Thanks for the details! I did a similar test on my workstation and I'm getting the expected behavior: /dev/shm starts filling up when not using --disable-dev-shm-usage while /tmp/ keeps constant and vice-versa when using the flag. This is running from ToT, I'll check again once the change hits canary
,
Dec 14 2017
Sorry for the wild goose chase - you're right that the patch is working correctly. I'm not sure exactly what happened, but the build I downloaded from the link above earlier somehow didn't include your fix. When I downloaded the latest build once again just now, the switch works correctly. Thanks for your help and for the fix!
,
Dec 14 2017
Awesome! Thanks for double checking!
,
Dec 28 2017
David, thanks a lot for fixing this! Do you think the bug can be closed now?
,
Dec 29 2017
Is this still landing in 64.0.3281.0?
,
Jan 1 2018
It should be included in 65.0.3299.6 I'll mark this bug as fixed and create https://crbug.com/798221 to track using memfd_create in the future
,
Jan 2 2018
Just to check will this fix also apply to the GAE environment?
,
Jan 2 2018
Yep, you should be able to add the flag when running in GAE. Are you seeing issues?
,
Jan 18 2018
@dvallet, we're trying to figure out if we should add this flag by default for Puppeteer users on Linux. It would reduce the number of Docker issues we get :) https://github.com/GoogleChrome/puppeteer/issues/1834 Can you help me understand: - Are there implications/drawbacks when using the flag? - Why this isn't Chrome's new default behavior? - Does it work with headful Chrome?
,
Jan 19 2018
I think it would depend on how are /dev/shm and /tmp mounted. If they are both mounted as tmpfs I'm assuming there won't be any difference. if for some reason /tmp is not mapped as tmpfs (and I think is mapped as tmpfs by default by systemd), chrome shared memory management always maps files into memory when creating an anonymous shared files, so even in that case shouldn't be much difference. I guess you could force telemetry tests with the flag enabled and see how it goes. As for why not use by default, it was a pushed back by the shared memory team, I guess it makes sense it should be useing /dev/shm for shared memory by default. Ultimately all this should be moving to use memfd_create, but I don't think that's going to happen any time soon, since it will require refactoring Chrome memory management significantly.
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45a7b305282b175e11c871c1a8ac3b8336c66259 commit 45a7b305282b175e11c871c1a8ac3b8336c66259 Author: Wez <wez@chromium.org> Date: Thu Feb 22 22:42:39 2018 Fix CreateAnonymousSharedMemory() not to leak FILE when returning fd. CreateAnonymousSharedMemory() was modified to return the writable memory handle as a file-descriptor rather than as a FILE. Since POSIX does not provide a standard way to teardown a FILE without also close()ing the underlying file-descriptor, this was achieved by leaking the FILE. We now provide CreateAndOpenFdForTemporaryFileInDir(), to avoid the need to wrap the temporary-file descriptor into a FILE at all. Bug: 814444, 736452 Change-Id: Idd911f50f0e506de3eac91c809efb6c6d59fec10 Reviewed-on: https://chromium-review.googlesource.com/930336 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#538599} [modify] https://crrev.com/45a7b305282b175e11c871c1a8ac3b8336c66259/base/files/file_util.h [modify] https://crrev.com/45a7b305282b175e11c871c1a8ac3b8336c66259/base/files/file_util_posix.cc [modify] https://crrev.com/45a7b305282b175e11c871c1a8ac3b8336c66259/base/memory/shared_memory_helper.cc
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fca5d89ce30c4af1f53750de111e774b031f70f5 commit fca5d89ce30c4af1f53750de111e774b031f70f5 Author: Wez <wez@chromium.org> Date: Wed Feb 28 00:35:23 2018 Fix CreateAnonymousSharedMemory() not to leak FILE when returning fd. CreateAnonymousSharedMemory() was modified to return the writable memory handle as a file-descriptor rather than as a FILE. Since POSIX does not provide a standard way to teardown a FILE without also close()ing the underlying file-descriptor, this was achieved by leaking the FILE. We now provide CreateAndOpenFdForTemporaryFileInDir(), to avoid the need to wrap the temporary-file descriptor into a FILE at all. Bug: 814444, 736452 Change-Id: Idd911f50f0e506de3eac91c809efb6c6d59fec10 Reviewed-on: https://chromium-review.googlesource.com/930336 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#538599}(cherry picked from commit 45a7b305282b175e11c871c1a8ac3b8336c66259) Reviewed-on: https://chromium-review.googlesource.com/940163 Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#615} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/fca5d89ce30c4af1f53750de111e774b031f70f5/base/files/file_util.h [modify] https://crrev.com/fca5d89ce30c4af1f53750de111e774b031f70f5/base/files/file_util_posix.cc [modify] https://crrev.com/fca5d89ce30c4af1f53750de111e774b031f70f5/base/memory/shared_memory_helper.cc |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by aatr...@gmail.com
, Jul 12 2017