Commit 4f9ceb2f authored by erikchen's avatar erikchen Committed by Commit bot

Telemetry: Explicitly disconnect InspectorBackend.

Previously, the websocket was disconnected in __del__, whose timing was
non-deterministic. Now the websocket is disconnected as soon as its owner
(typically devtools_client_backend) no longer wants to keep track of it.

BUG=461153

Review URL: https://codereview.chromium.org/980043003

Cr-Commit-Position: refs/heads/master@{#319775}
parent a164bcfc
...@@ -100,9 +100,12 @@ def FindAllAvailableBrowsers(finder_options, device): ...@@ -100,9 +100,12 @@ def FindAllAvailableBrowsers(finder_options, device):
browser_types = set() browser_types = set()
for url in debug_urls: for url in debug_urls:
context = {'webSocketDebuggerUrl': url, 'id': 1} context = {'webSocketDebuggerUrl': url, 'id': 1}
inspector = inspector_backend.InspectorBackend( try:
backend.app, backend.devtools_client, context) inspector = inspector_backend.InspectorBackend(
res = inspector.EvaluateJavaScript("navigator.userAgent") backend.app, backend.devtools_client, context)
res = inspector.EvaluateJavaScript("navigator.userAgent")
finally:
inspector.Disconnect()
match_browsers = re.search(browser_pattern, res) match_browsers = re.search(browser_pattern, res)
if match_browsers: if match_browsers:
browser_types.add(match_browsers.group(1)) browser_types.add(match_browsers.group(1))
......
...@@ -30,6 +30,11 @@ def _IsDevToolsAgentAvailable(devtools_http_instance): ...@@ -30,6 +30,11 @@ def _IsDevToolsAgentAvailable(devtools_http_instance):
class DevToolsClientBackend(object): class DevToolsClientBackend(object):
"""An object that communicates with Chrome's devtools.
This class owns a map of InspectorBackends. It is responsible for creating
them and destroying them.
"""
def __init__(self, devtools_port, remote_devtools_port, app_backend): def __init__(self, devtools_port, remote_devtools_port, app_backend):
"""Creates a new DevToolsClientBackend. """Creates a new DevToolsClientBackend.
...@@ -194,6 +199,8 @@ class _DevToolsContextMapBackend(object): ...@@ -194,6 +199,8 @@ class _DevToolsContextMapBackend(object):
context_ids = [context['id'] for context in contexts] context_ids = [context['id'] for context in contexts]
for context_id in self._inspector_backends_dict.keys(): for context_id in self._inspector_backends_dict.keys():
if context_id not in context_ids: if context_id not in context_ids:
backend = self._inspector_backends_dict[context_id]
backend.Disconnect()
del self._inspector_backends_dict[context_id] del self._inspector_backends_dict[context_id]
valid_contexts = [] valid_contexts = []
......
...@@ -34,13 +34,19 @@ def _HandleInspectorWebSocketExceptions(func): ...@@ -34,13 +34,19 @@ def _HandleInspectorWebSocketExceptions(func):
def inner(inspector_backend, *args, **kwargs): def inner(inspector_backend, *args, **kwargs):
try: try:
return func(inspector_backend, *args, **kwargs) return func(inspector_backend, *args, **kwargs)
except (socket.error, websocket.WebSocketException) as e: except (socket.error, websocket.WebSocketException,
inspector_websocket.WebSocketDisconnected) as e:
inspector_backend._ConvertExceptionFromInspectorWebsocket(e) inspector_backend._ConvertExceptionFromInspectorWebsocket(e)
return inner return inner
class InspectorBackend(object): class InspectorBackend(object):
"""Class for communicating with a devtools client.
The owner of an instance of this class is responsible for calling
Disconnect() before disposing of the instance.
"""
def __init__(self, app, devtools_client, context, timeout=60): def __init__(self, app, devtools_client, context, timeout=60):
self._websocket = inspector_websocket.InspectorWebsocket() self._websocket = inspector_websocket.InspectorWebsocket()
self._websocket.RegisterDomain( self._websocket.RegisterDomain(
...@@ -68,8 +74,17 @@ class InspectorBackend(object): ...@@ -68,8 +74,17 @@ class InspectorBackend(object):
self._network = inspector_network.InspectorNetwork(self._websocket) self._network = inspector_network.InspectorNetwork(self._websocket)
self._timeline_model = None self._timeline_model = None
def Disconnect(self):
"""Disconnects the inspector websocket.
This method intentionally leaves the self._websocket object around, so that
future calls it to it will fail with a relevant error.
"""
if self._websocket:
self._websocket.Disconnect()
def __del__(self): def __del__(self):
self._websocket.Disconnect() self.Disconnect()
@property @property
def app(self): def app(self):
......
...@@ -8,8 +8,13 @@ import logging ...@@ -8,8 +8,13 @@ import logging
import socket import socket
import time import time
from telemetry.core import exceptions
from telemetry.core.backends.chrome_inspector import websocket from telemetry.core.backends.chrome_inspector import websocket
class WebSocketDisconnected(exceptions.Error):
"""An attempt was made to use a web socket after it had been disconnected."""
pass
class InspectorWebsocket(object): class InspectorWebsocket(object):
...@@ -71,9 +76,12 @@ class InspectorWebsocket(object): ...@@ -71,9 +76,12 @@ class InspectorWebsocket(object):
"""Sends a request without waiting for a response. """Sends a request without waiting for a response.
Raises: Raises:
websocket.WebSocketException websocket.WebSocketException: Error from websocket library.
socket.error socket.error: Error from websocket library.
exceptions.WebSocketDisconnected: The socket was disconnected.
""" """
if not self._socket:
raise WebSocketDisconnected()
req['id'] = self._next_request_id req['id'] = self._next_request_id
self._next_request_id += 1 self._next_request_id += 1
data = json.dumps(req) data = json.dumps(req)
...@@ -85,12 +93,13 @@ class InspectorWebsocket(object): ...@@ -85,12 +93,13 @@ class InspectorWebsocket(object):
"""Sends a request and waits for a response. """Sends a request and waits for a response.
Raises: Raises:
websocket.WebSocketException websocket.WebSocketException: Error from websocket library.
socket.error socket.error: Error from websocket library.
exceptions.WebSocketDisconnected: The socket was disconnected.
""" """
self.SendAndIgnoreResponse(req) self.SendAndIgnoreResponse(req)
while self._socket: while True:
res = self._Receive(timeout) res = self._Receive(timeout)
if 'id' in res and res['id'] == req['id']: if 'id' in res and res['id'] == req['id']:
return res return res
...@@ -99,8 +108,9 @@ class InspectorWebsocket(object): ...@@ -99,8 +108,9 @@ class InspectorWebsocket(object):
"""Waits for responses from the websocket, dispatching them as necessary. """Waits for responses from the websocket, dispatching them as necessary.
Raises: Raises:
websocket.WebSocketException websocket.WebSocketException: Error from websocket library.
socket.error socket.error: Error from websocket library.
exceptions.WebSocketDisconnected: The socket was disconnected.
""" """
self._Receive(timeout) self._Receive(timeout)
...@@ -110,9 +120,10 @@ class InspectorWebsocket(object): ...@@ -110,9 +120,10 @@ class InspectorWebsocket(object):
self._cur_socket_timeout = timeout self._cur_socket_timeout = timeout
def _Receive(self, timeout=10): def _Receive(self, timeout=10):
self._SetTimeout(timeout)
if not self._socket: if not self._socket:
return None raise WebSocketDisconnected()
self._SetTimeout(timeout)
data = self._socket.recv() data = self._socket.recv()
result = json.loads(data) result = json.loads(data)
if logging.getLogger().isEnabledFor(logging.DEBUG): if logging.getLogger().isEnabledFor(logging.DEBUG):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment