1141679 - Marionette doesn't clean up its socket following a restart initiated inside the application

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

Henrik describes the issue here: https://bugzilla.mozilla.org/show_bug.cgi?id=1130220#c4

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: bz://1141679/chmanchester

/r/5073 - Bug 1141679 - Ignore the server's response when quitting the application with marionette.

Pull down this commit:

hg pull review -r b1dce20b90468610467e9b3198eb45ba968a9baa

Comment 2

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/5071/#review4093

::: testing/marionette/client/marionette/tests/unit/test_profile_management.py
(Diff revision 1)
> -        self.assertTrue(bool_value)
> +        self.assertRaisesRegexp(JavascriptException, "Error getting pref",

I'm not 100% sure why this changes with the patch, but this isn't really a desired behavior in the first place, so whether the user pref's value isn't saved or it's not recognized at all may not matter.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8575527 [details]
MozReview Request: bz://1141679/chmanchester

/r/5073 - Bug 1141679 - Ignore the server's response when quitting the application with marionette.

Pull down this commit:

hg pull review -r b1dce20b90468610467e9b3198eb45ba968a9baa

Comment 4

[Chris Manchester (limited bugmail, email directly)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04

Comment 5

[Jonathan Griffin (:jgriffin)]

https://reviewboard.mozilla.org/r/5071/#review4099

> I'm not 100% sure why this changes with the patch, but this isn't really a desired behavior in the first place, so whether the user pref's value isn't saved or it's not recognized at all may not matter.

The new behavior seems correct.  I'm not sure how to explain the old behavior the test was verifiying.

Comment 6

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8575527 [details]
MozReview Request: bz://1141679/chmanchester

https://reviewboard.mozilla.org/r/5071/#review4101

Ship It!

Comment 7

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester [:chmanchester] from comment #4)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04

This is failing on osx in ways I can't explain.

Comment 8

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester [:chmanchester] from comment #7)
> (In reply to Chris Manchester [:chmanchester] from comment #4)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04
> 
> This is failing on osx in ways I can't explain.

Surely a bad build. Here it is again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=73edbe5e41c2

Comment 9

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester [:chmanchester] from comment #8)
> (In reply to Chris Manchester [:chmanchester] from comment #7)
> > (In reply to Chris Manchester [:chmanchester] from comment #4)
> > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04
> > 
> > This is failing on osx in ways I can't explain.
> 
> Surely a bad build. Here it is again:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=73edbe5e41c2

Here's the contents of gecko.log from the second bad build (same as the first): 

19:32:14     INFO -  dyld: Symbol not found: _strndup
19:32:14     INFO -    Referenced from: /builds/slave/talos-slave/test/build/application/NightlyDebug.app/Contents/MacOS/libmozglue.dylib
19:32:14     INFO -    Expected in: flat namespace
19:32:14     INFO -   in /builds/slave/talos-slave/test/build/application/NightlyDebug.app/Contents/MacOS/libmozglue.dylib

I'm having a very hard time believing this patch caused that, but I guess I can't really ask for checkin without getting try to pass.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thank you Chris for taking care of it that quickly. Lets hope we can figure out the remaining problem soon. Until then I will add our blocked whiteboard entry.

Comment 11

[Mats Palmgren (inactive)]

FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
before I found this bug.  Feel free to dupe it if you will handle it here.
It would be nice if you could look into that soon, for obvious reasons :-)

Comment 12

[Chris Manchester (limited bugmail, email directly)]

(In reply to Mats Palmgren (:mats) from comment #11)
> FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
> before I found this bug.  Feel free to dupe it if you will handle it here.
> It would be nice if you could look into that soon, for obvious reasons :-)

I'm certainly going to try to figure out what's going on but that bug seems more on topic. This isn't my area of expertise so I really don't know how much I'm going to be able to offer on the subject.

Comment 13

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester [:chmanchester] from comment #12)
> (In reply to Mats Palmgren (:mats) from comment #11)
> > FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
> > before I found this bug.  Feel free to dupe it if you will handle it here.
> > It would be nice if you could look into that soon, for obvious reasons :-)
> 
> I'm certainly going to try to figure out what's going on but that bug seems
> more on topic. This isn't my area of expertise so I really don't know how
> much I'm going to be able to offer on the subject.

I guess this is more likely bug 1141745. I'm just going to land this when inbound opens.

Comment 14

[Chris Manchester (limited bugmail, email directly)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6039462e84b0

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6039462e84b0

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We kinda would like to have this support for Firefox 38 to be able to run our Firefox UI tests. Would it be possible to backport this patch?

Comment 17

[Chris Manchester (limited bugmail, email directly)]

The two dependent follow ups should be uplifted with this bug.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Chris. I marked both as affected for 38.

Comment 19

[David Burns :automatedtester]

https://hg.mozilla.org/releases/mozilla-aurora/rev/75069e9797d9

Comment 21

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1141679 - Ignore the server's response when quitting the application with marionette.