Closed Bug 1240601 Opened 4 years ago Closed

Use a socket timeout in transport.wait_for_port

Categories

(Testing :: Marionette, defect)

defect
Not set

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

(Keywords: pi-marionette-client)

Attachments

(1 file)

The socket used in transport.wait_for_port never times out if it is able to connect but unable to receive any data.

We should use a relatively short timeout period on this socket.

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/transport/marionette_transport/transport.py?from=transport.py#287
Assignee: nobody → ato
Status: NEW → ASSIGNED
If the client is able to connect but the server never sends any data,
the socket should time out in order for the function not to hang forever.

That said, polling for ports like this is inherently racy and we should
instead specify and bind what port we wish Marionette to start on in
the future.

Review commit: https://reviewboard.mozilla.org/r/31323/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31323/
Attachment #8709185 - Flags: review?(dburns)
Attachment #8709185 - Flags: review?(dburns) → review+
Comment on attachment 8709185 [details]
MozReview Request: Bug 1240601 - Set timeout on socket in transport.wait_for_port; r?automatedtester

https://reviewboard.mozilla.org/r/31323/#review28177
This is what might have caused the coalesced backout of bug 1240576, bug 1240751, bug 1240610, and bug 1240601.
Comment on attachment 8709185 [details]
MozReview Request: Bug 1240601 - Set timeout on socket in transport.wait_for_port; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31323/diff/1-2/
Comment on attachment 8709185 [details]
MozReview Request: Bug 1240601 - Set timeout on socket in transport.wait_for_port; r?automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31323/diff/2-3/
I’m not seeing any failures from running the tests locally.  Maybe the backouts were caused by a race condition in how the patches were applied to central?
https://hg.mozilla.org/mozilla-central/rev/5c139df92844
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.