Bug 174220 - [GTK][WPE] kill-old-process should kill more webkit related process
Summary: [GTK][WPE] kill-old-process should kill more webkit related process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-06 15:13 PDT by Carlos Alberto Lopez Perez
Modified: 2017-07-06 17:47 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.90 KB, patch)
2017-07-06 15:25 PDT, Carlos Alberto Lopez Perez
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>