Bug 174220

Summary: [GTK][WPE] kill-old-process should kill more webkit related process
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: Tools / TestsAssignee: Carlos Alberto Lopez Perez <clopez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, lforschler, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mcatanzaro: review+

Description Carlos Alberto Lopez Perez 2017-07-06 15:13:28 PDT
kill-old-processes is executed on the bots before each new test run to kill leftover process from the previous run.
This helps to avoid that process that have become unresponsive to interfere with the next run of the bot (like leftover process consuming CPU or RAM)

The list of process to kill on Linux ports misses several process like jsc, and WebKitWebProcess, WebKitDatabaseProcess, etc...

I started to build a list of possible process, but then I realized there are much more than i imagined: for example, when for the gtk api tests there are dozens of different Test*API different process names. The names of some process also change between ports (GTK+ and WPE)

So it looks like a good idea to dynamically generate the possible names of all webkit-related process by looking for the binary names on the bin build directory and add that names to the list of process to kill
Comment 1 Carlos Alberto Lopez Perez 2017-07-06 15:25:56 PDT
Created attachment 314763 [details]
Patch
Comment 2 Michael Catanzaro 2017-07-06 17:32:57 PDT
Comment on attachment 314763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314763&action=review

> Tools/BuildSlaveSupport/kill-old-processes:28
> +def listAllWebKitPrograms(build_dir_bin):

Nit: I might have named this builddir_bin instead.

> Tools/BuildSlaveSupport/kill-old-processes:32
> +            if os.access(os.path.join(root, file), os.X_OK):

Why do you need to call access?

> Tools/BuildSlaveSupport/kill-old-processes:136
> +        build_dir_bin = "WebKitBuild/Release/bin" if os.path.isdir("WebKitBuild/Release/bin") else "WebKitBuild/Debug/bin"

I was going to object to this, but I guess it would be pretty odd if the built binaries ever differ between release and debug builds, so this seems fine.
Comment 3 Carlos Alberto Lopez Perez 2017-07-06 17:40:01 PDT
(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 314763 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=314763&action=review
> 
> > Tools/BuildSlaveSupport/kill-old-processes:28
> > +def listAllWebKitPrograms(build_dir_bin):
> 
> Nit: I might have named this builddir_bin instead.
> 

Looks better, will change it.

> > Tools/BuildSlaveSupport/kill-old-processes:32
> > +            if os.access(os.path.join(root, file), os.X_OK):
> 
> Why do you need to call access?
> 

To check if the file is executable, otherwise I don't want to add its filename to the list of possible webkit-related process.

> > Tools/BuildSlaveSupport/kill-old-processes:136
> > +        build_dir_bin = "WebKitBuild/Release/bin" if os.path.isdir("WebKitBuild/Release/bin") else "WebKitBuild/Debug/bin"
> 
> I was going to object to this, but I guess it would be pretty odd if the
> built binaries ever differ between release and debug builds, so this seems
> fine.

The idea is to get WebKitBuild/Release/bin on release bots and WebKitBuild/Debug/bin on debug bots. All linux ports (GTK+, WPE and JSCOnly) use one of the two possibilities to store the binaries that will be executed (depending if its a release or a debug bot).
Comment 4 Carlos Alberto Lopez Perez 2017-07-06 17:47:02 PDT
Committed r219227: <http://trac.webkit.org/changeset/219227>