win: Only capture the loader lock for now

win: Only capture the loader lock for now

On Win 7 in a debug configuration, walking all locks was gathering
hundreds of thousands of locks, causing test timeouts to be exceeded in
debug. On user machines, UnhandledExceptionHandler() could have timed
out too. For now, only grab the loader lock as it's the most interesting
one. Unfortunately, this means that !locks won't work for now.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/1f3ced1846f3956e678c562c90e0d6c970543617

[scottmg, 2015-11-26 20:19:12]

(I'm going to temporarily land and revert this to be able to run tryjobs with
this inside Chrome.)

[scottmg, 2015-11-26 20:19:33]

Description was changed from

==========
win: Cap number of locks gathered

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288
==========

to

==========
win: Cap number of locks gathered

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

[scottmg, 2015-11-26 20:19:34]

Committed patchset #1 (id:1) manually as
b3464d96f5fc0d82f860651b7918626dfbd80d65 (presubmit successful).

[scottmg, 2015-11-26 20:19:44]

Description was changed from

==========
win: Cap number of locks gathered

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

to

==========
win: Cap number of locks gathered

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

[scottmg, 2015-11-26 20:23:42]

On 2015/11/26 20:19:34, scottmg wrote:
> Committed patchset #1 (id:1) manually as
> b3464d96f5fc0d82f860651b7918626dfbd80d65 (presubmit successful).

Reverted here: https://codereview.chromium.org/1474223002

[Mark Mentovai, 2015-11-30 18:32:06]

> On Win 7 in a debug configuration, this was gathering hundreds of

Didn’t happen in release? Are the locks in release mode allocated without debug
info?

…

Perhaps a strategy for the initial landing is to not collect any extra
information relative to Breakpad (yet), wait a few days to see what happens to
crash rates on the canary, and then start enabling the new stuff.

https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
File snapshot/win/process_snapshot_win.cc (right):

https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
snapshot/win/process_snapshot_win.cc:333: ReadLocks<Traits>(peb_data.LoaderLock,
&extra_memory_);
If we really only care about the loader lock, maybe we should just do this one
and #if 0 out the next one until we have a better mechanism to control it.

https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
snapshot/win/process_snapshot_win.cc:452: const int kMaxWalkLength = 100;
…and maybe we should just grab the loader lock here instead of grabbing an
arbitrary number of other locks.

[scottmg, 2015-11-30 19:42:40]

Description was changed from

==========
win: Cap number of locks gathered

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

to

==========
win: Only capture the loader lock for now

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

[scottmg, 2015-11-30 19:43:55]

Description was changed from

==========
win: Only capture the loader lock for now

On Win 7 in a debug configuration, this was gathering hundreds of
thousands of locks, causing test timeouts to be exceeded. On user
machines, UnhandledExceptionHandler() probably would have timed out
also. Arbitrarily cap the number of locks captured, as we don't have a
pressing need for anything other than the LoaderLock anyway.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/b3464d96f5fc0d82f860651...
==========

to

==========
win: Only capture the loader lock for now

On Win 7 in a debug configuration, walking all locks was gathering
hundreds of thousands of locks, causing test timeouts to be exceeded in
debug. On user machines, UnhandledExceptionHandler() could have timed
out too. For now, only grab the loader lock as it's the most interesting
one. Unfortunately, this means that !locks won't work for now.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288
==========

[scottmg, 2015-11-30 19:50:27]

On 2015/11/30 18:32:06, Mark Mentovai wrote:
> > On Win 7 in a debug configuration, this was gathering hundreds of
> 
> Didn’t happen in release? Are the locks in release mode allocated without
debug
> info?

Sorry, that was badly worded. It was happening in Release too, it just didn't
take forever.

> 
> …
> 
> Perhaps a strategy for the initial landing is to not collect any extra
> information relative to Breakpad (yet), wait a few days to see what happens to
> crash rates on the canary, and then start enabling the new stuff.

Yeah, sounds reasonable. Unfortunately it breaks !locks, but you can still view
the loader lock via `!cs ntdll!LdrpLoaderLock` for now.

> 
>
https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
> File snapshot/win/process_snapshot_win.cc (right):
> 
>
https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
> snapshot/win/process_snapshot_win.cc:333:
ReadLocks<Traits>(peb_data.LoaderLock,
> &extra_memory_);
> If we really only care about the loader lock, maybe we should just do this one
> and #if 0 out the next one until we have a better mechanism to control it.
> 
>
https://codereview.chromium.org/1475033005/diff/1/snapshot/win/process_snapsh...
> snapshot/win/process_snapshot_win.cc:452: const int kMaxWalkLength = 100;
> …and maybe we should just grab the loader lock here instead of grabbing an
> arbitrary number of other locks.

[Mark Mentovai, 2015-11-30 20:27:08]

LGTM

[scottmg, 2015-11-30 20:29:27]

Description was changed from

==========
win: Only capture the loader lock for now

On Win 7 in a debug configuration, walking all locks was gathering
hundreds of thousands of locks, causing test timeouts to be exceeded in
debug. On user machines, UnhandledExceptionHandler() could have timed
out too. For now, only grab the loader lock as it's the most interesting
one. Unfortunately, this means that !locks won't work for now.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288
==========

to

==========
win: Only capture the loader lock for now

On Win 7 in a debug configuration, walking all locks was gathering
hundreds of thousands of locks, causing test timeouts to be exceeded in
debug. On user machines, UnhandledExceptionHandler() could have timed
out too. For now, only grab the loader lock as it's the most interesting
one. Unfortunately, this means that !locks won't work for now.

In the future, we may want to figure out a signalling mechanism so that
the client can note other interesting locks to be grabbed, and just
avoid walking the list entirely.

R=mark@chromium.org
BUG=chromium:546288

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/1f3ced1846f3956e678c562...
==========

[scottmg, 2015-11-30 20:29:28]

Committed patchset #2 (id:20001) manually as
1f3ced1846f3956e678c562c90e0d6c970543617 (presubmit successful).