Restarting Chrome within the browser on Linux causes an increasing chain of cat processes to be launched |
||
Issue descriptionChrome Version: 63.0.3239.108 OS: Ubuntu 14.04 What steps will reproduce the problem? (1) Launch Chrome (2) pgrep -c cat (3) chrome://restart (Or restart via about:flags or the update restart prompt) (4) pgrep -c cat (5) Repeat steps (3) and (4) until bored. What is the expected result? The number of cat processes should not increase over time. What happens instead? Every time Chrome restarts, the number of cat processes increases by two. This is caused by the fix for issue #376567 . When we restart the browser, Chrome makes a new child process based on its previous argv just before exitting: https://cs.chromium.org/chromium/src/chrome/browser/browser_shutdown.cc?l=232 However, the wrapper script uses exec -a, so Chrome thinks its argv[0] is the wrapper rather than the actual binary. https://cs.chromium.org/chromium/src/chrome/installer/linux/common/wrapper?rcl=30a800ab092d8a2c24aee9e9ed73e55352ad1970&l=49 This causes it to run the wrapper script again on restart, which dutifully chains another cat in front of each stdout and stderr. As the browser self-restarts, stdout and stderr get a longer and longer cat pipeline, until it is closed normally for whatever reason. One possibility is to drop the exec -a bit so self-restarts don't re-run the wrapper. The consequences I can see there are: 1. On update, we'd maybe want to re-run the wrapper, in case there's anything new in there?? 2. The browser process will look different in ps... if we care. Another is to remove the cats and instead fix issue #376567 by having the browser process manage the stdout/stderr pipes and remember to undo it before restarting. But sounds like a lot of work. Another is we could maybe detect if stdout/stderr is already a pipe (not sure how one does that in shell short of /proc...) and not inject a cat in that case? Dunno if there are any cases where that would be bad. mdempsky doesn't seem to work on Chrome anymore, so CCing thomasanderson for Linux stuff and folks from the original bug who still work in the vague vicinity or may find this entertaining. (Note there appears to be a second copy of the wrapper script now, so any changes there should be mirrored: https://cs.chromium.org/chromium/src/extensions/shell/installer/linux/common/wrapper?rcl=30a800ab092d8a2c24aee9e9ed73e55352ad1970&l=32 ) Please use labels and text to provide additional information. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Dec 22 2017
Mike for Linux knowledge as well. I'd say restart without the wrapper when restarting from the browser.
,
Dec 22 2017
Oh hrm. Another benefit of restarting without the wrapper is that we also prepend more and more copies of $HERE:$HERE/lib to LD_LIBRARY_PATH on restarts. I am worried about weird bugs coming from restarts across the wrapper changing though. (Something truly horrific we could do is have the wrapper set CHROME_WRAPPER_VERSION=N and have it skip modifications on restart, but that seems nuts...)
,
Dec 22 2017
I'd say restart without the wrapper, see if anything breaks.
,
Dec 22 2017
I'm OK with restarting without the wrapper, so long as we add some comments in the wrapper warning about changing LD_LIBRARY_PATH etc.
,
Dec 22 2017
SGTM. Really the only things that absolutely need to go into the wrapper script are stuff that'll prevent launching the executable. After that, we can do things in C++ just fine. And, worst case, if we find we absolutely need to rerun some wrapper code on restart, we can move chrome to chrome.real and make chrome itself into a wrapper script too. (Hrm. Any reason why the LD_LIBRARY_PATH business isn't an RPATH $ORIGIN in the executable?)
,
Dec 22 2017
> (Hrm. Any reason why the LD_LIBRARY_PATH business isn't an RPATH $ORIGIN in the executable?) Bug 556843 has some details on this. Apparently it can open some odd security holes related to setuid sandboxing.
,
Dec 22 2017
Ah. Though that doesn't preclude us building chrome with RPATH $ORIGIN and chrome-sandbox (which should have extremely minimal dependencies anyway) without. *shrug* Doesn't hugely matter. The wrapper script works. |
||
►
Sign in to add a comment |
||
Comment 1 by davidben@chromium.org
, Dec 22 2017