On Mon, Oct 19, 2015 at 02:09:01PM +0200, Daniel Vetter wrote: > On Thu, Oct 15, 2015 at 11:22:45AM -0700, Dylan Baker wrote: > > On Thu, Oct 15, 2015 at 11:45:57AM +0100, Thomas Wood wrote: > > > On 14 October 2015 at 22:35, Dylan Baker <[email protected]> wrote: > > > > On Wed, Oct 14, 2015 at 04:27:56PM +0100, Thomas Wood wrote: > > > >> On 9 October 2015 at 20:53, <[email protected]> wrote: > > > >> > From: Dylan Baker <[email protected]> > > > >> > > > >> The process is not killed when the timeout occurs, so the previous > > > >> code to implement that needs to be added here. This involves sending > > > >> SIGTERM to the child process and then sending SIGKILL to the process > > > >> group if SIGTERM failed to remove the process. The child processes > > > >> were placed in their own process group by the preexec_fn parameter > > > >> being set to __set_process_group. > > > >> > > > > > > > > If you have subprocess32 it will be killed when the timeout expires. > > > > This is a backport of the python3 behavior, from the python3 docs: > > > > > > > > """ > > > > The timeout argument is passed to Popen.communicate(). If the timeout > > > > expires, the child process will be killed and waited for. The > > > > TimeoutExpired exception will be re-raised after the child process has > > > > terminated. > > > > """ > > > > > > > > Am I missing something here? > > > > > > I think that is the documentation for subprocess.run, which has a > > > slightly different behaviour to Popen.communicate: > > > > > > https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate > > > > > > "The child process is not killed if the timeout expires, so in order > > > to cleanup properly a well-behaved application should kill the child > > > process and finish communication:" > > > > > > > The docs recommend using proc.kill() for this purpose. > > > > I'm really hesitant to use preexec_fn, because there is a big warning > > that preexec_fn is not safe to use with threads, and for the Open{G,C}L > > tests having concurrency is a requirement, so if we require preexec_fn > > we make timeouts and concurency mutually exclusive. > > > > Looking at the docs there may be a way to handle process groups without > > the use of a preexec_fn. I'll look into it further. > > > > > > > > The additional step of sending SIGKILL to the process group also helps > > > to clean up misbehaving tests that have forked. > > > > > > > Do IGT tests regularly fork? I don't think any of the native piglit > > tests fork. > > They fork a _lot_. Some do for concurrency stress tests, others so that > you have a parent who can watch the child die (e.g. to check that we > correctly clean up all the crap left behind by a dying process using drm). > > We also use fork to send signals to the main thread, which is used to > validate the ioctl restarting. > -Daniel
Okay. I have a solution that I think will work for igt, I just need to figure out how to de-tangle stuff enough to not break windows when we want to turn this on for wider piglit. Dylan
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
