1083131 - Marionette does not always remove temporary profiles

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

mozrunner produces temporary folders in /tmp that are not cleaned up.  I haven't investigated if this happens for clean exits, interrupts, terminations, or all of these.

This morning the mozrunner related directories in /tmp had grown to ~790M.

Comment 1

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

Which type of tests were causing this behavior? What are those files? The profile data?

Comment 2

[Andreas Tolfsen ❲:ato❳]

They look like directories containing the profile data as you say.  They appear to be generated from running `./mach marionette-test`.

~ % lc -d /tmp/*mozrunner
tmp0ybIF_.mozrunner tmp8kX8cz.mozrunner tmpBqyKwK.mozrunner tmpNvnqZq.mozrunner tmpWs5pYO.mozrunner tmpbxBEq3.mozrunner tmpi1XoFm.mozrunner tmplGIFFP.mozrunner tmpqJpz9u.mozrunner
tmp11nnL8.mozrunner tmp9MWuWG.mozrunner tmpLVuEhI.mozrunner tmpS8EOr4.mozrunner tmpWve283.mozrunner tmpc248KF.mozrunner tmpieS1wE.mozrunner tmplcvvw8.mozrunner tmptK6WUk.mozrunner
tmp6N9XAy.mozrunner tmp9swc2l.mozrunner tmpMQgvsJ.mozrunner tmpTBa0SY.mozrunner tmpXigj1a.mozrunner tmpd9jcmO.mozrunner tmpj8RCk1.mozrunner tmpmRCyAI.mozrunner tmpusH4ed.mozrunner
tmp6_VBVs.mozrunner tmpA2WUoJ.mozrunner tmpMvXVZq.mozrunner tmpTol_Lm.mozrunner tmpXzLin0.mozrunner tmpda1XSi.mozrunner tmpl28Hg0.mozrunner tmpoA6BJ2.mozrunner tmpwiwgsr.mozrunner
tmp7LOBKV.mozrunner tmpBai08l.mozrunner tmpN50YK_.mozrunner tmpUxnK88.mozrunner tmpaIwKiJ.mozrunner tmpdsXPUB.mozrunner tmplBcNVR.mozrunner tmpooSArl.mozrunner tmpwrbgSf.mozrunner
tmp7SpHBt.mozrunner tmpBhSrAO.mozrunner tmpNtpstG.mozrunner tmpVkp9_G.mozrunner tmpbJizkh.mozrunner tmpgmGBOv.mozrunner tmplBcZdt.mozrunner tmppCR24L.mozrunner
~ % lc /tmp/tmp0ybIF_.mozrunner 
.parentlock             compatibility.ini       extensions.ini          minidumps               prefs.js                sessionstore-backups    webappsstore.sqlite
blocklist.xml           content-prefs.sqlite    extensions.json         permissions.sqlite      safebrowsing            startupCache            webappsstore.sqlite-shm
bookmarkbackups         cookies.sqlite          key3.db                 places.sqlite           search.json             thumbnails              webappsstore.sqlite-wal
cache2                  crashes                 lock                    places.sqlite-shm       secmod.db               user.js
cert8.db                directoryLinks.json     mimeTypes.rdf           places.sqlite-wal       sessionCheckpoints.json webapps

Comment 3

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

Yeah, that seems to be the case. It would be good to know if that is always happening for you. You may also wanna run mozrunner yourself, and check afterward if the created profile is still present.

It may be that some code in Marionette doesn't let mozrunner remove the profile.

Comment 4

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

I actually did this myself, and all is fine for the mozrunner command line. But when running Marionette client I can see that the newly created profile is still present in the temporary folder. So this is clearly a Marionette issue.

Comment 6

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

This is also a blocker of our upcoming Firefox ui tests. Adding dependency for bug 1080766.

Comment 7

[Julien Pagès (:parkouss)]

Ok, I found the cause:

It appears that we are setting:

profile_args["restore"] = False

here:

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#72

And that cause the profile to not be deleted (I set it to True, and temp dirs are deleted). According to the doc:

:param restore: Flag for removing all custom settings during cleanup

So it seems *normal* to have this behaviour, since we set that to False. The question is why was this set to False ?

Comment 8

[Julien Pagès (:parkouss)]

Hm, I think that this piece of code (in the context given in comment 7):

if hasattr(self, "profile_path") and self.profile is None:
    if not self.profile_path:
        profile_args["restore"] = False

Could be translated by something like "if the profile given as an argument was None (or at least not evaluated to True), then set restore=False". This is because "profile_path" is only defined if the profile argument was not a Profile instance, and in this case we set profile_path = profile (in our case, None).

Does that make sense ? Maybe we should just get rid of this 'profile_args["restore"] = False' line.

BTW, all the logic here with profile/profile_path/profile_args seems tricky and a bit broken. I'm not sure to understand what we wanted to do with this code.

Comment 9

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

Thank you Julien for investigating the problem! I kinda appreciate that. I'm also a bit new to Marionette, so I don't know the details about that code and the expected behavior.

David, can you give us some more information here how Marionette handles profiles?

Comment 10

[David Burns :automatedtester]

Marionette uses mozrunner and mozprofile to start the browser into a state where we can automate things. 

see https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#55. We merge them together on start()

Comment 11

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

Ok, so given that you can pass in an existing profile via the --profile option, we should only remove the profile if it has really been created by Marionette. Or do we copy the existing profile to a temporarily location? Maybe that is what we have here?

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#76

So if we always create a new profile, we should indeed remove it at every time.

Comment 12

[Chris Manchester (limited bugmail, email directly)]

Removing the 'profile_args["restore"] = False' line quoted in comment 8 fixes this when I tried it. This appears to be the correct behavior -- this is the common case no profile was passed in and a fresh one was created. If we need a mode for "post mortem" debugging a profile created during the test run I suppose that could be added.

Comment 13

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: bz://1083131/chmanchester

/r/7135 - Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato

Pull down this commit:

hg pull -r eef071a3398c65a2150febbc2b0e67e4a47e1271 https://reviewboard-hg.mozilla.org/gecko/

Comment 14

[Chris Manchester (limited bugmail, email directly)]

Anyhow, there's a patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=55ce1b1fb237

Comment 15

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester

https://reviewboard.mozilla.org/r/7133/#review5933

Ship It!

Comment 16

[Chris Manchester (limited bugmail, email directly)]

Aha. Windows.

Comment 17

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester [:chmanchester] from comment #16)
> Aha. Windows.

The only platform where removing the profile while the browser is running is considered a fatal error. This looks ok locally: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93a35acfe73b

Comment 18

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester

/r/7135 - Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato

Pull down this commit:

hg pull -r d63b9ac77254a08b73507c706e44545b5df095c4 https://reviewboard-hg.mozilla.org/gecko/

Comment 19

[Chris Manchester (limited bugmail, email directly)]

Ignore the review request if try in comment 17 isn't ok, but this fixes things on a windows build locally.

Comment 20

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester

https://reviewboard.mozilla.org/r/7133/#review5959

Ship It!

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/778e99b46052

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/778e99b46052

Comment 25

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato