New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 610613 link

Starred by 2 users
Status: Assigned
Owner:
█ tansell@chromium.org
Last visit > 30 days ago
Cc:
█ kbr@chromium.org
█ jparent@chromium.org
jam@chromium.org
█ phajdan.jr@chromium.org
jsb...@chromium.org
dpranke@chromium.org
iannu...@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Task

Blocked on:
issue 610614
issue 610618
Blocking:
issue 694326



Sign in to add a comment

Audit usage of subprocess in run-webkit-tests to discover any issues with threads

Project Member Reported by tansell@chromium.org, May 10 2016 
In the bug at https://bugs.chromium.org/p/chromium/issues/detail?id=522396 we discovered that the following code will lock up on Windows;

----------------------------
 p = subprocess.Popen(...., stdout=subprocess.PIPE)
 
 def reader(pipe):
   while True:
     data = pipe.read(x)
     if not data:
       break

 t1 = threading.Thread(run=reader, args=(p.stdout))

 t1.join()
----------------------------

We should check that nothing in run-webkit-tests uses this model.

Comment 1 by tansell@chromium.org, May 10 2016 

tansell@tansell-z620-l2:/fast/chrome/src$ grep -R "import subprocess" third_party/WebKit/*                                                                                                                          
third_party/WebKit/PRESUBMIT.py:    import subprocess
third_party/WebKit/Source/bindings/scripts/utilities.py:import subprocess
third_party/WebKit/Source/build/scripts/rule_bison.py:import subprocess
third_party/WebKit/Source/build/scripts/make_css_property_names.py:import subprocess
third_party/WebKit/Source/build/scripts/make_css_value_keywords.py:import subprocess
third_party/WebKit/Source/build/scripts/gperf.py:import subprocess
third_party/WebKit/Source/devtools/scripts/compiler-runner/build_compiler_runner_jar.py:import subprocess
third_party/WebKit/Source/devtools/scripts/optimize_png_images.py:import subprocess
third_party/WebKit/Source/devtools/scripts/compile_frontend.py:import subprocess
third_party/WebKit/Source/devtools/scripts/jsdoc-validator/run_tests.py:import subprocess
third_party/WebKit/Source/devtools/scripts/jsdoc-validator/build_jsdoc_validator_jar.py:import subprocess
third_party/WebKit/Source/devtools/scripts/convert_svg_images_to_png.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/tool/servers/layouttestsserver.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/common/multiprocessing_bootstrap.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/common/system/path.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/common/system/user.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive_unittest.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftest.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/bisect_test_ordering.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/server_process.py:    import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/android.py:import subprocess
third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/sslutils/openssl.py:import subprocess

Comment 2 by tansell@chromium.org, May 10 2016 

I'm starting with the ones under "third_party/WebKit/Tools/Scripts/webkitpy/common/"

Comment 3 by tansell@chromium.org, May 10 2016

Blockedon: 610614

Comment 4 by tansell@chromium.org, May 10 2016

Blockedon: 610618

Comment 5 Deleted

Comment 6 by sshru...@google.com, May 18 2016

Labels: Test-Layout

Comment 7 by sshru...@google.com, May 18 2016

Components: -Blink>LayoutTests Blink
Deprecating component:Blink>LayoutTests, to use label Test=Layout instead. Merging these to component:Blink for the Blink rotation to pick up and re-triage as appropriate.

Comment 8 by cbiesin...@chromium.org, May 23 2016

Components: -Blink Blink>Infra

Comment 9 by qyears...@chromium.org, Dec 2 2016

Cc: kbr@chromium.org
Note: In  bug 637578 , kbr noted that subprocess42 (https://cs.chromium.org/chromium/infra/luci/client/utils/subprocess42.py) may be a better replacement for subprocess in some places.

Also, a note about where to investigate first: Most sub-processes created by run-webkit-tests are created via Executive.run_command (https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py?l=348)

Comment 10 by tansell@chromium.org, Feb 20 2017

Blocking: 694326

Comment 11 by qyears...@chromium.org, Aug 9 2017

Labels: -Type-Bug -Test-Layout Hotlist-CodeHealth Type-Task

Comment 12 by iannu...@google.com, Oct 19

Cc: -iannucci@chromium.org iannu...@google.com
►

Sign in to add a comment