[rfc][icedtea-web] Kill processes in Integration Tests more cleanly

[Jie Kang]

Hello,

It turns out midori enters 'restore from crash' mode after multiple SIGUSR1s. Manually opening midori, sending SIGUSR1 and reopening midori doesn't trigger this though :( I'm also a little surprised that it didn't do this with SIGINT but okay..

Here is the revised patch that uses SIGTERM instead of SIGUSR1. If worst comes to worst then switching back to SIGINT would be fine too. I also noticed that the previous patch had another error, there was a 'p.destroy()' call that was removed but then added in another place. This call has been removed, as it should've been.


I tested this with firefox, midori and epiphany on Fedora 21. I haven't been able to get opera on my system yet :\

Thoughts?


Regards,



----- Original Message -----
> On 04/09/2015 09:58 PM, Jie Kang wrote:
> > Hello,
> >
> > This patch does four things:
> >
> > 1. Removes the usage of the softkiller : it no longer closes tabs properly
> > and requires very old 3rd-party dependencies
> > 2. Changes the signal sent from SIGINT to SIGUSR1 : mainly for firefox
> > which doesn't handle any of the signals very well :
> > https://bugzilla.mozilla.org/show_bug.cgi?id=336193
> >      Afaik the other browsers handle this signal by closing. I haven't been
> >      able to find any documentation on whether or not they close
> >      gracefully though.
> >      SIGTERM would be okay too
> > 3. Removes the usage of p.destroy() : it may or may not forcibly kill the
> > process depending on the implementation and with the signal sent above,
> > shouldn't be necessary
> > 4. Lowers the timeout time from 20 seconds to 10 seconds
> >
> > In general, it should make the suite go faster and remove issues that come
> > from the browser and applet being destroyed without letting them clean up.
> >
> > I have run the reproducer suite on my machine (F21, openjdk 1.8) and there
> > haven't been any new failing tests while these changes were applied.
> > Technically, my timeout was at 5 seconds, not 10, but I think 10 is more
> > reasonable given that some of the tests involve downloading from a remote
> > source. To be honest though, I'd prefer it if each individual test was
> > able to run in under 5 seconds...
> >
> > If others could take this patch, apply it and see how the reproducers fair
> > for them, it'd be greatly appreciated.
> >
> >
> Only two nits
> 
> Are you sure with timeout? I'm quite unsure about it... Well, lets try...
> 
> Please remove the whole build and sources and depndencies from configure
> related to softkiller.
> 
> 
> You may do it in two changsets, as I would rather rereview the removal.
> 
> 
> So ok to push for this one. And please prepare the removal.
> 
> 
> 
> I had not tested it, but Iknow you have beenstruggling with it for pretty lng
> time. So guess it
> really helps. Also get rid of not standart killer would be probably useful.
> 
> ty!
> J.
> 
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern
-------------- next part --------------
A non-text attachment was scrubbed...
Name: itw-integration-kill-cleanly-2.patch
Type: text/x-patch
Size: 3062 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150413/b31d64ac/itw-integration-kill-cleanly-2.patch>