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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
