1358987 - Marionette server is not getting started when update.status file of Firefox gets modified before a restart

Status: RESOLVED WORKSFORME

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Henrik Skupin (:whimboo) from bug 1358402 comment #2)
> Created attachment 8860294 [details]
> minimized testcase
> 
> So I found a minimal testcase to reproduce this problem with the beta
> releases of Firefox. As you can see no download of an update patch is
> necessary. Just modifying the `update.status` file before a restart causes
> Marionette not to be started anymore.

(In reply to Henrik Skupin (:whimboo) from bug 1358402 comment #4)
> Actually this regression has been caused by bug 1344748, and is still
> present for latest mozilla-central when executed with Firefox builds now on
> Beta.
> 
> Steps to reproduce:
> 
> 1. Download any of the builds from [1] or build yourself from changeset
> cf76e00dcd6f142acf5b49f8beeb0ac95b2afa31 on mozilla-beta
> 
> 2. Run the minimized testcase against this build.
> 
> [1] here:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> beta&revision=cf76e00dcd6f142acf5b49f8beeb0ac95b2afa31&filter-
> searchStr=build&filter-tier=1&filter-tier=2&filter-tier=3

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Henrik Skupin (:whimboo) from bug 1358402 comment #14)
> So since my changes to bug 1315611, Marionette also uses
> "sessionstore-windows-restored" to initialize itself. So starting its server
> component will be some milliseconds later. But nothing in the
> _postUpdateProcessing() code shows something which might prevent Marionette
> from starting up.
> 
> Matt, is there somewhere an extra hidden restart which we do in such a
> failed case (invalid patch as listed in update.status)? Given that I can get
> it to work by adding back this old preference means, that for such a restart
> we loose the -marionette argument. It would somewhat be an indication for me
> because without modifying the file the restart works.

Btw in bug 1355888 we will make use of an environment variable to keep the status of Marionette across Firefox instances. So it might fix such a situation at least for builds of Firefox 55.0.

Comment 2

[Molly Howell (she/her) (mostly inactive)]

Oh. Yeah, if sessionstore-windows-restored is when Marionette gets initialized, then that could easily be causing problems, because it might just not be running yet when we show the update wizard. I had assumed starting Marionette would happen earlier than that. It looks from bug 1355888 that it does happen earlier than that on the first run, but then the command-line parameter that caused that to happen gets lost? If my understanding is correct there, I would expect that new environment variable to make a big difference in bug 1358402.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

(In reply to Matt Howell [:mhowell] from comment #2)
> Oh. Yeah, if sessionstore-windows-restored is when Marionette gets
> initialized, then that could easily be causing problems, because it might
> just not be running yet when we show the update wizard. I had assumed
> starting Marionette would happen earlier than that. It looks from bug

It doesn't hurt us when the update wizard window opens before Marionette gets started. We will still get it and switch to it before continuing.

> 1355888 that it does happen earlier than that on the first run, but then the
> command-line parameter that caused that to happen gets lost? If my
> understanding is correct there, I would expect that new environment variable
> to make a big difference in bug 1358402.

So I would say lets wait until the environment variable patch got landed. Then we can see if that improves / fixes this bug.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

This seems to work all fine those days:

1500239160146	Marionette	TRACE	6 <- [1,17,null,{"cause":"restart"}]
1500239160185	Marionette	DEBUG	Closed connection 6
1500239160186	Marionette	WARN	New connections are currently not accepted
1500239160186	addons.productaddons	WARN	Failed downloading via XHR, status: 0, reason: error
1500239160731	Marionette	DEBUG	Received observer notification "xpcom-shutdown"
1500239160731	Marionette	DEBUG	Received observer notification "xpcom-shutdown"
1500239161418	Marionette	INFO	Enabled via MOZ_MARIONETTE
1500239161418	Marionette	DEBUG	Received observer notification "profile-after-change"
1500239161442	Marionette	DEBUG	Received observer notification "command-line-startup"
1500239162559	Marionette	DEBUG	Received observer notification "sessionstore-windows-restored"

So nothing we would have to worry about.