New issue
Advanced search Search tips

Issue 601143 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Crashpad handler has a handle to an old version directory

Project Member Reported by grt@chromium.org, Apr 6 2016

Issue description

51.0.2701.1 (Official Build) canary SyzyASan (32-bit)

The commandline is:

"C:\Users\me\AppData\Local\Google\Chrome SxS\Application\chrome.exe" --type=crashpad-handler /prefetch:7 --no-rate-limit "--database=C:\Users\me\AppData\Local\Google\Chrome SxS\User Data\Crashpad" --url=https://clients2.google.com/cr/report --annotation=channel=canary --annotation=plat=Win32 --annotation=prod=Chrome --annotation=ver=51.0.2701.1 --handshake-handle=0xd8

It has a handle to the previous version's version dir, which may prevent the installer from deleting it.
 
Crashpad.png
4.8 KB View Download
Hmm, I'm not sure where that would be happening.

I notice on a non-upgrade restart there's still two handles to the version directory, but they're just the current one, not one old and one new.

Other chrome.exe children also have those two handles, here's a renderer:

Directory, \KnownDlls, 0x4c
Directory, \Sessions\1\BaseNamedObjects, 0x118
File, C:\Users\scott\AppData\Local\Google\Chrome SxS\Application\51.0.2701.0, 0x58
File, C:\Users\scott\AppData\Local\Google\Chrome SxS\Application\51.0.2701.0, 0x8c
...
Yeah, they're really early. If I run Canary (2701) from windbg with the startup dir set to C:\FAKE_STARTUP_DIR, I get these handles.

File, C:\FAKE_STARTUP_DIR, 0x18
File, C:\Users\scott\AppData\Local\Google\Chrome SxS\Application\51.0.2701.0, 0x5c

This is at process create in windbg, i.e. way before we've run any code.

1:002> k
 # Child-SP          RetAddr           Call Site
00 000000a4`706ff1a8 00007ffe`4485eaee ntdll!NtMapViewOfSection+0x14
01 000000a4`706ff1b0 00007ffe`4485e77e ntdll!LdrpMapViewOfSection+0xbe
02 000000a4`706ff250 00007ffe`4485e5ed ntdll!LdrpMapImage+0x72
03 000000a4`706ff2f0 00007ffe`44857bfa ntdll!LdrpMapDllWithSectionHandle+0x2d
04 000000a4`706ff330 00007ffe`44859537 ntdll!LdrpMapDllNtFileName+0x29a
05 000000a4`706ff400 00007ffe`448592dc ntdll!LdrpMapDllFullPath+0xcb
06 000000a4`706ff580 00007ffe`4487f89c ntdll!LdrpProcessWork+0x50
07 000000a4`706ff5d0 00007ffe`448e0013 ntdll!LdrpDrainWorkQueue+0x108
08 000000a4`706ff610 00007ffe`4491166d ntdll!LdrpInitializeProcess+0x1adf
09 000000a4`706ffa00 00007ffe`448c6d5e ntdll!_LdrpInitialize+0x4a8b9
0a 000000a4`706ffa80 00000000`00000000 ntdll!LdrInitializeThunk+0xe


So, my guess is that we're relaunching chrome with the cwd set to the old version directory.

Is that possible? Make any sense? Required?
Looking through calls to SetCurrentDirectory, it seems like it's probably https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/main_dll_loader_win.cc&l=60.

So... I think somewhere near https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/first_run/upgrade_util_win.cc&l=83 we probably need to SetCurrentDirectory() out of there. WDYT?
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20920cb19ec396276f3e772072e659fdb8f39ae0

commit 20920cb19ec396276f3e772072e659fdb8f39ae0
Author: scottmg <scottmg@chromium.org>
Date: Thu Apr 07 16:23:59 2016

Set the working directory on relaunch

The working directory seems to be set during DLL load. Since we don't set one
here, I think the end result is that that's inherited to the relaunched child.

R=grt@chromium.org
BUG= 601143 

Review URL: https://codereview.chromium.org/1862203004

Cr-Commit-Position: refs/heads/master@{#385778}

[modify] https://crrev.com/20920cb19ec396276f3e772072e659fdb8f39ae0/chrome/browser/first_run/upgrade_util_win.cc

Darn, my half-assed hopeful change didn't work, I still get

File, C:\Users\scott\AppData\Local\Google\Chrome SxS\Application\51.0.2701.0, 0x18
File, C:\Users\scott\AppData\Local\Google\Chrome SxS\Application\51.0.2703.0, 0x44

on updating from 2701 to 2703. I'll try to figure out how to actually test this locally. :/

Comment 7 by grt@chromium.org, Apr 8 2016

I thought so too when I first looked this morning, but I think we won't know until tomorrow. Your CWD change needs to be in the old version that is relaunching in favor of the new version, right? So hold off on looking for a different culprit until Monday.
Ah, good point, thanks. I'll take a look at the next Canary.

Comment 9 by grt@chromium.org, Apr 11 2016

Status: Fixed (was: Assigned)
Look like it worked. My relaunch to 2705 has the handles looking good:

File, C:\Users\me\AppData\Local\Google\Chrome SxS\Application, 0x20
File, C:\Users\me\AppData\Local\Google\Chrome SxS\Application\52.0.2705.1, 0x24

Thanks, Scott!

Sign in to add a comment