win ninja: Speculative fix for pdb locking errors

win ninja: Speculative fix for pdb locking errors

There's longstanding occasional errors where the compiler complains that
the pdb is locked. This isn't really a condition that should be exposed
to the build system, and no one has been able to come up with a reliable
repro. Retrying causes the build to succeed.

In chromium, two targets that often fail this way are 'net' and
'libphonenumber_without_metadata'. These targets are distinctive in that
they build early in the build, contain at least one .c file, and use
pch for their .cc files.

The locking error seems to manifest with the compile step for the obj
associated with creating the .pdb, so the hypothesis is that the .c
compilation is colliding with the creation of the pdb that the
precompile does. This race should be handled by mspdbsrv, but for
appears to be buggy for whatever reason. (This bug has been filed on
connect.microsoft.com, but without a repro it's not very likely that
action will be taken.)

In order to avoid this, have the .c and .cc files use separate .pdb
files. This is otherwise unnecessary, but shouldn't cause any material
drawback either.

R=thakis@chromium.org
BUG=chromium:142362
Committed: https://code.google.com/p/gyp/source/detail?r=1800

[Nico, 2013-11-27 00:11:52]

Nice! Can this theory be converted into a somewhat reliable repro case?

Bug link for the connect.ms.com bug?

[Nico, 2013-11-27 00:13:19]

https://codereview.chromium.org/88303004/diff/20001/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/88303004/diff/20001/pylib/gyp/generator/ninja...
pylib/gyp/generator/ninja.py:817: pdbpath_c = pdbpath_cc =
self.msvs_settings.GetCompilerPdbName(
Comment why there are two pdbs (link to review comment or bug, or just a summary
of the cl description)

https://codereview.chromium.org/88303004/diff/20001/pylib/gyp/generator/ninja...
pylib/gyp/generator/ninja.py:1807: # By making the rules target separate pdb
files this might be avoided.
Oh, here it is. The just a comment saying "# See comment above cc_command for
why there are two pdb files" up there is fine too.

[scottmg, 2013-11-27 00:29:53]

On 2013/11/27 00:11:52, Nico wrote:
> Nice! Can this theory be converted into a somewhat reliable repro case?

Sadly no. I tried what I thought was a good test over night last night, but it
didn't fail unfortunately. So either the timing window is very short, or the
theory is totally wrong, I'm not sure.

> 
> Bug link for the http://connect.ms.com bug?

There's a few, but they're all kind of confused. Perhaps it's worth filing
another. They'll close-without-reading anything that's filed against VS2010
though and we don't know if it happens on VS2013 yet for lack of deployment.

http://connect.microsoft.com/VisualStudio/feedback/details/329604/error-c2471...

http://stackoverflow.com/a/127103

http://social.msdn.microsoft.com/Forums/vstudio/en-US/34e8105d-7cfe-4b74-af92...

[scottmg, 2013-11-27 00:32:31]

https://codereview.chromium.org/88303004/diff/20001/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/88303004/diff/20001/pylib/gyp/generator/ninja...
pylib/gyp/generator/ninja.py:1807: # By making the rules target separate pdb
files this might be avoided.
On 2013/11/27 00:13:19, Nico wrote:
> Oh, here it is. The just a comment saying "# See comment above cc_command for
> why there are two pdb files" up there is fine too.

Done.

[Nico, 2013-11-27 00:48:43]

Thanks, lgtm. Definitley worth a try, but if it's hard to turn this into a repro
I'm not super hopeful.

[scottmg, 2013-11-27 00:57:04]

On 2013/11/27 00:48:43, Nico wrote:
> Thanks, lgtm. Definitley worth a try, but if it's hard to turn this into a
repro
> I'm not super hopeful.

Yeah, I'm going to try for a couple more hours on the repro. I noticed they also
added a vague statement to the /FS documentation
http://msdn.microsoft.com/en-us/library/vstudio/dn502518.aspx that sort of says
you just *might* have problems if you compile multiple files at the same time.
:/

"""
This may make builds significantly longer, and it doesn't prevent all errors
that may occur when multiple instances of cl.exe access the PDB file at the same
time. We recommend that you change your solution so that independent projects
write to separate intermediate and output locations, or that you make one of the
projects dependent on the other to force serialized project builds.
"""

I don't know of a practical way to set up the build to not use /FS though. In
ninja-speak they basically want a pool of depth = 1 for each pdb. That might be
OK for clean builds, but it'd be poor if changing only files in one target, as
they'd all build serially.

[scottmg, 2013-11-27 00:58:14]

Committed patchset #3 manually as r1800.