Bug 218643 - REGRESSION(r268930): It broke the http server of run-benchmark
Summary: REGRESSION(r268930): It broke the http server of run-benchmark
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: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-05 20:09 PST by Carlos Alberto Lopez Perez
Modified: 2020-11-13 12:02 PST (History)
8 users (show)

See Also:


Attachments
Patch (4.23 KB, patch)
2020-11-05 20:17 PST, Carlos Alberto Lopez Perez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (6.12 KB, patch)
2020-11-05 20:48 PST, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (6.38 KB, patch)
2020-11-05 20:54 PST, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (6.38 KB, patch)
2020-11-06 06:20 PST, Carlos Alberto Lopez Perez
no flags 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 2020-11-05 20:09:44 PST
On https://build.webkit.org/builders/GTK-Linux-64-bit-Release-Perf/builds/10332/steps/benchmark-test/logs/stdio the benchmarks are all failing with:

Traceback (most recent call last):
  File "/home/buildbot/worker/gtk-linux-64-release-perf-tests/build/Tools/Scripts/webkitpy/browserperfdash/browserperfdash_runner.py", line 169, in run
    runner.execute()
  File "/home/buildbot/worker/gtk-linux-64-release-perf-tests/build/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py", line 120, in execute
    self._run_benchmark(int(self._plan['count']), web_root)
  File "/home/buildbot/worker/gtk-linux-64-release-perf-tests/build/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py", line 90, in _run_benchmark
    result = self._run_one_test(web_root, self._plan['entry_point'])
  File "/home/buildbot/worker/gtk-linux-64-release-perf-tests/build/Tools/Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py", line 43, in _run_one_test
    raise error
AccessDenied: psutil.AccessDenied (pid=12593)


This is caused because the http server launched via a subprocess exits as soon as is launched with an import error.
So then the call to  "psutil.Process(self._server_process.pid).connections()" tries to access the PID of a zombie in /proc causing the AccessDenied error.

The http server exits with this error:

Traceback (most recent call last):
  File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/http_server/twisted_http_server.py", line 17, in <module>
    from webkitpy.autoinstalled import twisted
ImportError: No module named webkitpy.autoinstalled


Which in turn is caused because Tools/Scripts is not on PYTHONPATH and it can't find webkitpy.autoinstalled
Comment 1 Carlos Alberto Lopez Perez 2020-11-05 20:17:18 PST
Created attachment 413390 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2020-11-05 20:48:45 PST
Created attachment 413395 [details]
Patch

v2: refactor the loop to avoid repeating the code twice
Comment 3 Carlos Alberto Lopez Perez 2020-11-05 20:54:44 PST
Created attachment 413396 [details]
Patch

cosmetic changes: remove empty line
Comment 4 Carlos Alberto Lopez Perez 2020-11-06 06:20:30 PST
Created attachment 413428 [details]
Patch

fix style errors
Comment 5 Jonathan Bedard 2020-11-06 08:17:50 PST
Comment on attachment 413428 [details]
Patch

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

> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:39
> +        for attempt in range(max_attempt):

While these changes aren't required to fix the regression, they do seem to be an improvement to readability.
Comment 6 Carlos Alberto Lopez Perez 2020-11-06 08:46:36 PST
(In reply to Jonathan Bedard from comment #5)
> Comment on attachment 413428 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413428&action=review
> 
> > Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:39
> > +        for attempt in range(max_attempt):
> 
> While these changes aren't required to fix the regression, they do seem to
> be an improvement to readability.

Right.. my first patch was not doing this changes.. but after I uploaded I looked at it and it looked ugly enough to have to repeat the code twice that I ended doing this small changes to try to improve the code quality

Thanks for the review!
Comment 7 Carlos Alberto Lopez Perez 2020-11-06 09:08:12 PST
Comment on attachment 413428 [details]
Patch

Clearing flags on attachment: 413428

Committed r269514: <https://trac.webkit.org/changeset/269514>
Comment 8 Carlos Alberto Lopez Perez 2020-11-06 09:08:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2020-11-06 09:09:25 PST
<rdar://problem/71121500>