It's a copy of http://codereview.chromium.org/155772, with the fix for valgri...

It's a copy of http://codereview.chromium.org/155772, with the fix for valgrind test failure.

This CL implements the second TODO item of issue 12343:
2) We should send back an ACK to the second process.  If the second process doesn't get an ACK in the given timeout, it should kill the first process and go ahead and start.

The approach of this CL is to append process id to the singleton's socket filename, such as "SingletonSocket-12345", and creates a symbol link "SingletonSocket" to the real socket file. In ProcessSingleton::NotifyOtherProcess() if it's successfully connected to "SingletonSocket" but no ACK received, then the original process can be killed by its process id retrieved from the symbol link.

BUG=12343
ProcessSingleton Linux cleanups
TEST=In one terminal, launch chrome and stop the process by pressing ctrl-z, then launch chrome again in another terminal. The second chrome shall be started in 5 seconds, and the first one shall be killed.

[James Su, 2009-07-31 03:16:32]

Hi,
  I just uploaded this CL again with my chromium account. The valgrind issue was
fixed by adding sleep(1) before and after calling NotifyOtherProcess() in
NotifyOtherProcessFailure test.
  If you think this CL is ready for submission, I'd like to submit it by myself
to test my new chromium account :-)
  We may address the NFS related issue in another CL.

Regards
James su

[willchan no longer on Chromium, 2009-07-31 04:15:36]

You should use the changes from http://codereview.chromium.org/159577 instead of
155772, because I made a few important fixes for you.  Both windows and mac
ports will fail to build (for the windows case, you need to remove the
definition of the constant.  for mac and windows, you need to make sure not to
build the linux ui test).  Also, linux fails to build due to the missing headers
(signal.h and another one I think).

You should test out the changes via the try servers.  Do gcl try <changelist
name>.

[James Su, 2009-07-31 04:39:08]

Sorry for my fault. I'll update this CL asap.

On 2009/07/31 04:15:36, willchan wrote:
> You should use the changes from http://codereview.chromium.org/159577 instead
of
> 155772, because I made a few important fixes for you.  Both windows and mac
> ports will fail to build (for the windows case, you need to remove the
> definition of the constant.  for mac and windows, you need to make sure not to
> build the linux ui test).  Also, linux fails to build due to the missing
headers
> (signal.h and another one I think).
> 
> You should test out the changes via the try servers.  Do gcl try <changelist
> name>.

[James Su, 2009-07-31 05:07:35]

new CL uploaded. I did gcl try before uploading the new CL. Seems that I should
do it afterwards.

[willchan no longer on Chromium, 2009-07-31 05:12:12]

lgtm

i'm about to take off soon, but feel free to commit.  please monitor
http://build.chromium.org/ to see if it breaks anything.  if you need to revert,
you can use depot_tools/revert.py to revert.  good luck with your first commit!

[James Su, 2009-07-31 07:05:23]

Unfortunately, this CL still causes ui test failure when running with valgrind.
Maybe sleeping 1 second is still not enough.

[James Su, 2009-07-31 08:22:40]

Hi,
  I changed the sleep timeout from 1 second to 10 seconds. But I found that 'gcl
try' won't execute valgrind tests. So I don't know if it can solve the problem.
  I tried valgrind ui_test on my machine without a problem.

Can you please help test it as well? It would be very bad if it still breaks
valgrind ui_test.

Regards
James Su

[James Su, 2009-08-03 07:30:48]

Hi,
  I just found the reason: when running ui_tests with valgrind, the browser
process is launched by a magic wrapper shell script instead of directly from
testing process. The process id returned by UITest::process() is the magic
wrapper's id. The browser process of course can't be stop by killing this id.
  I think we can only do for now is to disable this test, unless we find a way
to retrieve browser process's real id. What's your opinion about it?

Regards
James Su

On 2009/07/31 08:22:40, James Su wrote:
> Hi,
>   I changed the sleep timeout from 1 second to 10 seconds. But I found that
'gcl
> try' won't execute valgrind tests. So I don't know if it can solve the
problem.
>   I tried valgrind ui_test on my machine without a problem.
> 
> Can you please help test it as well? It would be very bad if it still breaks
> valgrind ui_test.
> 
> Regards
> James Su

[James Su, 2009-08-04 04:48:19]

Hi,
  I just migrated this CL to the latest trunk and fixed the valgrind ui_tests
issue. Please help review again.

Regards
James Su

[willchan no longer on Chromium, 2009-08-04 05:17:57]

lgtm!