Changeset 58434 in webkit


Ignore:
Timestamp:
Apr 28, 2010 2:51:34 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-28 Eric Seidel <eric@webkit.org>

Reviewed by David Levin.

Audit all uses of subprocess in webkitpy
https://bugs.webkit.org/show_bug.cgi?id=38284

After further discussions with Jeffrey Yasskin
about http://bugs.python.org/issue2320
and related issues of using subprocess from
multiple threads, I have learned that subprocess
is known to be non-threadsafe through recent
Python 2.7 builds.

I'm attempting to lessen our exposure to these
subprocess bugs by auditing each use of subprocess
in webkitpy. I did not find any unsafe calls
in my audit, but I did remove numerous unneeded
import subprocess lines.

  • Scripts/webkitpy/common/checkout/api.py:
  • Scripts/webkitpy/common/net/bugzilla.py:
  • Scripts/webkitpy/common/system/deprecated_logging_unittest.py:
  • Scripts/webkitpy/common/system/user.py:
  • Scripts/webkitpy/layout_tests/port/base.py:
  • Scripts/webkitpy/layout_tests/port/chromium_linux.py:
  • Scripts/webkitpy/layout_tests/port/chromium_mac.py:
  • Scripts/webkitpy/layout_tests/port/chromium_mac_unittest.py: Added.
  • Scripts/webkitpy/layout_tests/port/chromium_win.py:
  • Scripts/webkitpy/layout_tests/port/gtk.py:
  • Scripts/webkitpy/layout_tests/port/mac.py:
  • Scripts/webkitpy/layout_tests/port/qt.py:
  • Scripts/webkitpy/layout_tests/port/webkit.py:
  • Scripts/webkitpy/layout_tests/port/win.py:
Location:
trunk/WebKitTools
Files:
14 edited
1 copied

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r58411 r58434  
     12010-04-28  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by David Levin.
     4
     5        Audit all uses of subprocess in webkitpy
     6        https://bugs.webkit.org/show_bug.cgi?id=38284
     7
     8        After further discussions with Jeffrey Yasskin
     9        about http://bugs.python.org/issue2320
     10        and related issues of using subprocess from
     11        multiple threads, I have learned that subprocess
     12        is known to be non-threadsafe through recent
     13        Python 2.7 builds.
     14
     15        I'm attempting to lessen our exposure to these
     16        subprocess bugs by auditing each use of subprocess
     17        in webkitpy.  I did not find any unsafe calls
     18        in my audit, but I did remove numerous unneeded
     19        import subprocess lines.
     20
     21        * Scripts/webkitpy/common/checkout/api.py:
     22        * Scripts/webkitpy/common/net/bugzilla.py:
     23        * Scripts/webkitpy/common/system/deprecated_logging_unittest.py:
     24        * Scripts/webkitpy/common/system/user.py:
     25        * Scripts/webkitpy/layout_tests/port/base.py:
     26        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
     27        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
     28        * Scripts/webkitpy/layout_tests/port/chromium_mac_unittest.py: Added.
     29        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
     30        * Scripts/webkitpy/layout_tests/port/gtk.py:
     31        * Scripts/webkitpy/layout_tests/port/mac.py:
     32        * Scripts/webkitpy/layout_tests/port/qt.py:
     33        * Scripts/webkitpy/layout_tests/port/webkit.py:
     34        * Scripts/webkitpy/layout_tests/port/win.py:
     35
    1362010-04-28  Darin Adler  <darin@apple.com>
    237
  • trunk/WebKitTools/Scripts/webkitpy/common/checkout/api.py

    r58261 r58434  
    2828
    2929import os
    30 import subprocess
    3130import StringIO
    3231
  • trunk/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py

    r58210 r58434  
    3434import re
    3535import StringIO
    36 import subprocess
    3736
    3837from datetime import datetime # used in timestamp()
  • trunk/WebKitTools/Scripts/webkitpy/common/system/deprecated_logging_unittest.py

    r56699 r58434  
    2828
    2929import os
    30 import subprocess
    3130import StringIO
    3231import tempfile
  • trunk/WebKitTools/Scripts/webkitpy/common/system/user.py

    r58314 r58434  
    6363        editor = os.environ.get("EDITOR") or "vi"
    6464        args = shlex.split(editor)
     65        # Note: Not thread safe: http://bugs.python.org/issue2320
    6566        subprocess.call(args + files)
    6667
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py

    r58395 r58434  
    3636import os
    3737import shlex
    38 import subprocess
    3938import sys
    4039import time
     
    149148        result = True
    150149        try:
    151             if subprocess.call(cmd) == 0:
     150            if self._executive.run_command(cmd, return_exit_code=True) == 0:
    152151                return False
    153152        except OSError, e:
     
    156155            else:
    157156                raise e
    158         except ValueError:
    159             # work around a race condition in Python 2.4's implementation
    160             # of subprocess.Popen. See http://bugs.python.org/issue1199282 .
    161             pass
    162157        return result
    163158
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py

    r58397 r58434  
    3232import logging
    3333import os
    34 import platform
    3534import signal
    36 import subprocess
    3735
    3836import chromium
     
    123121                       'wdiff"')
    124122            _log.error('')
     123        # FIXME: The ChromiumMac port always returns True.
    125124        return result
    126125
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py

    r58397 r58434  
    3434import platform
    3535import signal
    36 import subprocess
    3736
    3837import chromium
     38
     39from webkitpy.common.system.executive import Executive
    3940
    4041_log = logging.getLogger("webkitpy.layout_tests.port.chromium_mac")
     
    100101
    101102    def _check_wdiff_install(self):
    102         # FIXME: This should use Executive.
    103         f = open(os.devnull, 'w')
    104         rcode = 0
    105103        try:
    106             rcode = subprocess.call(['wdiff'], stderr=f)
     104            # We're ignoring the return and always returning True
     105            self._executive.run_command([self._path_to_wdiff()], error_handler=Executive.ignore_error)
    107106        except OSError:
    108107            _log.warning('wdiff not found. Install using MacPorts or some '
    109108                         'other means')
    110             pass
    111         f.close()
    112109        return True
    113110
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac_unittest.py

    r58427 r58434  
    1 # Copyright (C) 2009 Google Inc. All rights reserved.
     1# Copyright (C) 2010 Google Inc. All rights reserved.
    22#
    33# Redistribution and use in source and binary forms, with or without
     
    2727# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    2828
    29 import os
    30 import subprocess
    31 import StringIO
    32 import tempfile
     29import chromium_mac
    3330import unittest
    3431
    35 from webkitpy.common.system.executive import ScriptError
    36 from webkitpy.common.system.deprecated_logging import *
    37 
    38 class LoggingTest(unittest.TestCase):
    39 
    40     def assert_log_equals(self, log_input, expected_output):
    41         original_stderr = sys.stderr
    42         test_stderr = StringIO.StringIO()
    43         sys.stderr = test_stderr
    44 
    45         try:
    46             log(log_input)
    47             actual_output = test_stderr.getvalue()
    48         finally:
    49             sys.stderr = original_stderr
    50 
    51         self.assertEquals(actual_output, expected_output, "log(\"%s\") expected: %s actual: %s" % (log_input, expected_output, actual_output))
    52 
    53     def test_log(self):
    54         self.assert_log_equals("test", "test\n")
    55 
    56         # Test that log() does not throw an exception when passed an object instead of a string.
    57         self.assert_log_equals(ScriptError(message="ScriptError"), "ScriptError\n")
     32from webkitpy.thirdparty.mock import Mock
    5833
    5934
    60 if __name__ == '__main__':
    61     unittest.main()
     35class ChromiumMacPortTest(unittest.TestCase):
     36
     37    def test_check_wdiff_install(self):
     38        port = chromium_mac.ChromiumMacPort()
     39        # Currently is always true, just logs if missing.
     40        self.assertTrue(port._check_wdiff_install())
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py

    r58397 r58434  
    3232import logging
    3333import os
    34 import platform
    35 import signal
    36 import subprocess
    3734import sys
    3835
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/gtk.py

    r58397 r58434  
    3131import logging
    3232import os
    33 import subprocess
    3433
    3534from webkitpy.layout_tests.port.webkit import WebKitPort
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py

    r58397 r58434  
    3131import logging
    3232import os
    33 import pdb
    3433import platform
    35 import re
    36 import shutil
    3734import signal
    38 import subprocess
    39 import sys
    40 import time
    41 import webbrowser
    4235
    4336import webkitpy.common.system.ospath as ospath
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py

    r58397 r58434  
    3131import logging
    3232import os
    33 import subprocess
    3433import signal
    3534
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py

    r58314 r58434  
    3636import logging
    3737import os
    38 import pdb
    39 import platform
    4038import re
    4139import shutil
    4240import signal
    43 import subprocess
    4441import sys
    4542import time
     
    389386        command += "\n"
    390387
    391         # pdb.set_trace()
    392388        self._server_process.write(command)
    393389
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/win.py

    r58397 r58434  
    3131import logging
    3232import os
    33 import subprocess
    3433
    3534from webkitpy.layout_tests.port.webkit import WebKitPort
Note: See TracChangeset for help on using the changeset viewer.