Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(288)

Issue 600002: blocking on sys_wait

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 5 months ago by issac.xwork
Modified:
3 years, 5 months ago
CC:
thestig1
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

in function ´╝ÜSuspendThread() (from  Linux_Ptrace_dump.cc)

In some occasion, process will be blocking on function call to sys_wait() 

the root cause is:  the parent process may have installed signal handler for
SIGCHLD, and calling waitpid() in that handler.
If that is the case , sys_wait() will always fail.

so , to fix this issue, we must ensure there is no such signal handler in the
process cloned from the parent process.

Patch Set 1 #

Total comments: 4

Patch Set 2 : as reviewer required. #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
exception_handler.cc View 1 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 8
issac.xwork
4 years, 5 months ago #1
issac.xwork
On 2013/05/24 17:02:15, issac.xwork wrote: anybody take a look at this issue please? or any ...
4 years, 5 months ago #2
Ted Mielczarek
Hi Isaac, This patch looks conceptually good, it just needs a little tweaking to be ...
4 years, 5 months ago #3
issac.xwork
https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception_handler.cc File src/client/linux/handler/exception_handler.cc (right): https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception_handler.cc#newcode389 src/client/linux/handler/exception_handler.cc:389: if(sigaction(SIGCHLD,NULL,&childsig_handler) == 0 && On 2013/06/06 13:53:36, Ted Mielczarek ...
4 years, 5 months ago #4
issac.xwork
as reviewer required.
4 years, 5 months ago #5
issac.xwork
as reviewer required.
4 years, 5 months ago #6
issac.xwork
On 2013/06/06 13:53:35, Ted Mielczarek wrote: > Hi Isaac, > > This patch looks conceptually ...
4 years, 5 months ago #7
issac.xwork
4 years, 2 months ago #8
Hi, Ted
Can you help review this once again?
Thanks.

On 2013/06/16 14:39:30, issac.xwork wrote:
> On 2013/06/06 13:53:35, Ted Mielczarek wrote:
> > Hi Isaac,
> > 
> > This patch looks conceptually good, it just needs a little tweaking to be
able
> > to land.
> > 
> >
>
https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception...
> > File src/client/linux/handler/exception_handler.cc (right):
> > 
> >
>
https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception...
> > src/client/linux/handler/exception_handler.cc:385: *  handler for SIGCHLD
will
> > make wait() hang forever.
> > Generally we avoid first-person in comments, although we're not totally
> > consistent about it. Also, capitalize the first word of sentences in
comments.
> > 
> >
>
https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception...
> > src/client/linux/handler/exception_handler.cc:389:
> > if(sigaction(SIGCHLD,NULL,&childsig_handler) == 0 &&
> > We avoid calling libc functions directly because of memory safety concerns.
> You
> > should be using the sys_ wrappers from linux_syscall_support.h, you can look
> > around the rest of this file for examples.
> > 
> > You should be able to simply use sys_sigaction and sys_sigemptyset here.
> > 
> >
>
https://breakpad.appspot.com/600002/diff/1/src/client/linux/handler/exception...
> > src/client/linux/handler/exception_handler.cc:396:
> > sigaction(SIGCHLD,&childsig_handler,NULL);
> > Your indentation here is weird.
> 
> Thank you for your review!
> I have uploaded a new patch as your review required.
> Please help review again.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted