Closed Bug 1083131 Opened 7 years ago Closed 6 years ago
Marionette does not always remove temporary profiles
MozReview Request: Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato
39 bytes, text/x-review-board-request
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.
Which type of tests were causing this behavior? What are those files? The profile data?
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
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.
Summary: mozrunner does not clean up temporary folders → mozrunner does not always clean-up temporary profiles
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.
Component: Mozbase → Marionette
QA Contact: hskupin
Summary: mozrunner does not always clean-up temporary profiles → Marionette does not always remove temporary profiles
This is also a blocker of our upcoming Firefox ui tests. Adding dependency for bug 1080766.
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 ?
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.
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?
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()
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.
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.
/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/
Attachment #8593518 - Flags: review?(ato)
Anyhow, there's a patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=55ce1b1fb237
Comment on attachment 8593518 [details] MozReview Request: bz://1083131/chmanchester https://reviewboard.mozilla.org/r/7133/#review5933 Ship It!
(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
Attachment #8593518 - Flags: review+ → review?(ato)
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/
Ignore the review request if try in comment 17 isn't ok, but this fixes things on a windows build locally.
Comment on attachment 8593518 [details] MozReview Request: bz://1083131/chmanchester https://reviewboard.mozilla.org/r/7133/#review5959 Ship It!
Assignee: nobody → cmanchester
You need to log in before you can comment on or make changes to this bug.