Marionette client doesn't use socket in a threadsafe way
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(firefox91 fixed)
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: jgraham, Assigned: jgraham)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
If we accidentially end up accessing the transport from multiple threads we can end up interleaving reads and so getting partial messages. It's probably reasonable to just fix this with a lock.
Assignee | ||
Comment 1•3 years ago
|
||
The previous function used early return in a way that made it hard to
follow, and made undocumented assumptions about the minimum possible
size of a message. This version is intended to have clearer control
flow and make the assumptions more explicit.
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
The GC is dangerous as we can end up calling the transport layer reentrantly,
resulting in corrupted packets. Since relying on gc for cleanup is often
unnecessary, put in an explicit cleanup argument that avoids doing work in the
destructor, and use it for wpt.
Assignee | ||
Comment 3•3 years ago
|
||
This should ensure that we can't end up with multiple threads
interleaving reads or writes on the socket.
Assignee | ||
Comment 4•3 years ago
|
||
This avoid the weirdness where we reach into the binary message data to
get the message type and then unconditionally deserialize as JSON anyway.
Updated•3 years ago
|
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/autoland/rev/995c3b9b36dc Rewrite the message recv function in marionette, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/02641f631413 Don't call cleanup from __del__ if it already ran, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/6b896d1cd3db Guard access to marionette socket with a lock, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/adb5fd73a3bc Deserialize marionette message earlier, r=webdriver-reviewers,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29465 for changes under testing/web-platform/tests
Comment 7•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/995c3b9b36dc
https://hg.mozilla.org/mozilla-central/rev/02641f631413
https://hg.mozilla.org/mozilla-central/rev/6b896d1cd3db
https://hg.mozilla.org/mozilla-central/rev/adb5fd73a3bc
Comment 8•3 years ago
|
||
Backed out 4 changesets (bug 1716963) for WPT failures in websockets/basic-auth.any.serviceworker.html?wpt_flags=h2. CLOSED TREE
Log:
https://treeherder.mozilla.org/logviewer?job_id=343683661&repo=autoland&lineNumber=29287
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=adb5fd73a3bc39a4886061aa256afe84ff200061
Backout:
https://hg.mozilla.org/integration/autoland/rev/93a9dff7ca656bec2063eccbab95a3ba1ba33d25
Upstream PR was closed without merging
Assignee | ||
Comment 10•3 years ago
|
||
Updated•3 years ago
|
Comment 11•3 years ago
|
||
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/autoland/rev/9d5ec2a71554 Rewrite the message recv function in marionette, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/6b8537e78d74 Don't call cleanup from __del__ if it already ran, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/6dc6503a3664 Guard access to marionette socket with a lock, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/ccd3c31983d2 Deserialize marionette message earlier, r=webdriver-reviewers,whimboo
Upstream PR merged by moz-wptsync-bot
Comment 13•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9d5ec2a71554
https://hg.mozilla.org/mozilla-central/rev/6b8537e78d74
https://hg.mozilla.org/mozilla-central/rev/6dc6503a3664
https://hg.mozilla.org/mozilla-central/rev/ccd3c31983d2
Assignee | ||
Updated•3 years ago
|
Updated•1 year ago
|
Description
•