Memory leak (due to accidentally ignored `fdopen` result) causes OOM in glibc `fork`.
Reported by
edy.b...@gmail.com,
Jun 26 2018
|
||||||
Issue descriptionChrome Version : 64.0.3282.140 This issue was originally posted on the patchset that caused the regression - https://chromium-review.googlesource.com/c/chromium/src/+/781160/#message-6a6fa9251f422507a88181b72c1b817f176b378f - here follows a copy of that description: `fdopen` will allocate a `FILE` that is leaked, but that's only part of the problem. The glibc implementation of `fork` will touch all `FILE`s [0] in the child process, defeating CoW and causing the copy of a significant portion of the parent heap. Every time an `exec` is needed (e.g. for file open/save dialogs), the child process can easily OOM (given a long-running browser) inside `fork`, before it can get to the `exec`. On Linux at least, processes with names starting with "TaskScheduler" can be observed as they quickly grow their memory usage, and I believe a thread I've found on the Google Chrome Help Forum [1] refers to this exact problem, as well. In general, I would avoid `FILE` as much as possible, as implementations appear to be quite inefficient and potentially disruptive. [0] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/nptl/fork.c;h=ec56a827eba483b6a6e3226a8c4ba7e1199d10a5;hb=HEAD#l37 [1] https://productforums.google.com/forum/#!msg/chrome/IjeicWOeYlo/uRoHbdU6BQAJ
,
Jun 26 2018
,
Jun 26 2018
,
Jun 27 2018
,
Jun 27 2018
Thanks for the report! I'll try to investigate the issue, the original cl was reverted, but some stuff was left over, which might be causing the issue.
,
Jul 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/676d713909eee89c42a4eced9f3198b539762e7d commit 676d713909eee89c42a4eced9f3198b539762e7d Author: David Vallet <dvallet@chromium.org> Date: Mon Jul 09 02:05:27 2018 Remove fdopen(fd, "a+") request in SharedMemory::Create. The fd is already created as R/W and append, so I don't believe this is needed anymore. Bonus points: we don't leak a FILE* Bug: 856535 Change-Id: If482d45fb505c83f5a814e3d9d5e9ae17b71295d Reviewed-on: https://chromium-review.googlesource.com/1124727 Commit-Queue: David Vallet <dvallet@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#573199} [modify] https://crrev.com/676d713909eee89c42a4eced9f3198b539762e7d/base/memory/shared_memory_posix.cc
,
Jul 9
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by edy.b...@gmail.com
, Jun 26 2018