Fix race in VerifyStackReadWithMultipleThreads

Fix race in VerifyStackReadWithMultipleThreads

VerifyStackReadWithMultipleThreads would fail periodically
because one of the child threads was not in the expected state.
Now each thread sends an indication that is is running immediately
before entering the busy loop. There is still a possibility that
the parent process can stop a child thread before it i ready.
Having the parent sleep for 100ms seems to be sufficient to
avoid the race.

Signed-off-by: Chris Dearman <chris@mips.com>

[chrisdearman]

This patch is dependent on http://breakpad.appspot.com/377001/

[Ted Mielczarek]

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
File src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc (right):

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc:54: int pipefd
= *(int *)data;
Google C++ style frowns upon C-style casts, so you should use a static_cast
here.

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
File src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc (right):

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc:234: sleep(1);
I'm sympathetic to working around the race here because this is nearly
impossible to get right, but sleeping for a full second is going to make the
tests a lot slower. Can you usleep for 100ms or so and still make this work
mostly-reliably?

[chrisdearman]

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
File src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc (right):

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc:54: int pipefd
= *(int *)data;
On 2012/04/11 20:23:15, Ted Mielczarek wrote:
> Google C++ style frowns upon C-style casts, so you should use a static_cast
> here.

Done.

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
File src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc (right):

http://breakpad.appspot.com/377002/diff/1/src/client/linux/minidump_writer/li...
src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc:234: sleep(1);
On 2012/04/11 20:23:15, Ted Mielczarek wrote:
> I'm sympathetic to working around the race here because this is nearly
> impossible to get right, but sleeping for a full second is going to make the
> tests a lot slower. Can you usleep for 100ms or so and still make this work
> mostly-reliably?

usleep(100000) (100ms) ran 2000 iterations without any failures. The test is
reported to be taking ~200ms

usleep(10000) (10ms) failed after about 70 iterations

I ran the original version (1s sleep) for 70000 iterations without failure.

I wasn't very happy about this approach in the first place, but if you think the
shorter sleep is acceptable I'll upload a modified version

[Ted Mielczarek]

On 2012/04/11 22:40:54, chrisdearman wrote:
 > I wasn't very happy about this approach in the first place, but if you think
the
> shorter sleep is acceptable I'll upload a modified version

If you could cut it down to 250 or even 500ms and still get it mostly reliable
that would be much nicer than having a 1s pause in the unittest runs. I agree
that there's no great solution here, short of reworking the entire test to use
ptrace to step the process to the right state before dumping it.

[chrisdearman]

On 2012/04/12 11:36:41, Ted Mielczarek wrote:
> If you could cut it down to 250 or even 500ms and still get it mostly reliable
> that would be much nicer than having a 1s pause in the unittest runs. I agree
> that there's no great solution here, short of reworking the entire test to use
> ptrace to step the process to the right state before dumping it.

Patch Set 2 reduces the delay to 100ms and I haven't seen any failures with it
(yet...)

[Ted Mielczarek]

lgtm