blocking on sys_wait

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.

[issac.xwork]

On 2013/05/24 17:02:15, issac.xwork wrote:

anybody take a look at this issue please? or any comment?.

[Ted Mielczarek]

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.

[issac.xwork]

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:389:
if(sigaction(SIGCHLD,NULL,&childsig_handler) == 0 &&
On 2013/06/06 13:53:36, Ted Mielczarek wrote:
> 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.


ok, I noticed that before.
But I saw no use of sys_xxx like function for signal action, even in compromise
context, eg: code in ExceptionHandler::SignalHandler()

[issac.xwork]

as reviewer required.

[issac.xwork]

as reviewer required.

[issac.xwork]

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.

[issac.xwork]

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.