New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 856535 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit 29 days ago
Closed: Jul 9
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Memory leak (due to accidentally ignored `fdopen` result) causes OOM in glibc `fork`.

Reported by edy.b...@gmail.com, Jun 26 2018

Issue description

Chrome 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
 

Comment 1 by edy.b...@gmail.com, Jun 26 2018

The `FILE`-related inefficiency/OOM in the `fork` child process could also be avoided by using `posix_spawn` - which has been independently discussed at crbug.com/819228 and  crbug.com/179923 
Components: Internals>Core
Labels: Needs-Milestone
Owner: dvallet@chromium.org
Status: Assigned (was: Unconfirmed)
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.
Project Member

Comment 6 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)

Sign in to add a comment