org.mozilla.fennec is left running after dm.updateApp()

RESOLVED FIXED

Status

Testing
General
RESOLVED FIXED
8 years ago
6 months ago

People

(Reporter: bear, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
after the push-reboot-install process Fennec is still running and this is causing all tests to fail with a warning about an unclean system.

we confirmed this by calling dm.processExist('org.mozilla.fennec') and if the PID returned was not None, doing a dm.kill('org.mozilla.fennec') call.

After doing this we are seeing green test runs
(Reporter)

Updated

8 years ago
Blocks: 610600

Comment 1

8 years ago
I looked for /data/data/org.mozilla.fennec on tegras post-image after talking with Bob, and didn't find it on disk.

I think I was mistaken, and Bear had started tests on some of the newly imaged tegras, which installed fennec before I checked.  So now we're led to believe that org.mozilla.fennec is being started somewhere after installation.  I'm not sure if that's an SUT/helper bug or a Fennec bug.

We do have this workaround, but it would be good to solve the root problem and remove the workaround.
(Assignee)

Comment 2

8 years ago
I swear I have seen this locally, but nobody can confirm when I ask in IRC.  My impressions are this is a fennec specific thing after an initial first installation.

Comment 3

8 years ago
I've never seen this happen.  I could see how it could happen (perhaps if we had a pre-loader for fast start or something).  I don't think we have one of those in android (I think we did for winmo).  At any rate, given the unreproducibility of this it's probably far more deterministic to fight this with the workaround for now until we can find out exactly what is happening under the covers and figure out if that is something we can change or not.
(Assignee)

Comment 4

8 years ago
11:00 < jmaher> blassey: when rebooting a tegra, org.mozilla.fennec is running automatically.  Is this expected?
11:02 < mwu> jmaher: yes
11:03 < fabrice> jmaher, this is because we are notified when android.net.conn.CONNECTIVITY_CHANGE is broadcasted
11:03 < jmaher> mwu: ok, is this always going to be the case?  is there a way to disable this?
11:03 < mwu> jmaher: no easy way to disable it aside from removing the connectivity change listener from androidmanifest.xml
11:03 < jmaher> I can work around this, just want to make sure it is expected and we understand when to expect it
11:04 < jmaher> mwu: will this be on all devices (at least the majority)?
11:04 < mwu> jmaher: most real world devices will see that happen on startup


we should add some code in our scripts to kill fennec.
(Assignee)

Comment 5

8 years ago
Created attachment 509864 [details] [diff] [review]
kill fennec before starting mochitest/reftest (1.0)

simple patch to check for fennec running and kill it if needed.  This takes place in the test harness after parsing options and setting up, but before we do the profile creation/webserver setup/test running.

testing both harnesses with and without fennec running.
Assignee: nobody → jmaher
Attachment #509864 - Flags: review?(ctalbert)
(Assignee)

Comment 6

8 years ago
Created attachment 509972 [details] [diff] [review]
kill fennec before starting talos (1.0)

this patch for talos touches desktop code (although not specifically in the if() clause).  I tested this in a few cases on my tegra and it seems to work great.

I want good review of this and will need to run it through staging.
Attachment #509972 - Flags: review?(bear)
Attachment #509972 - Flags: feedback?(ctalbert)
(Assignee)

Updated

8 years ago
Attachment #509864 - Flags: feedback?(bear)
(Reporter)

Comment 7

8 years ago
Comment on attachment 509972 [details] [diff] [review]
kill fennec before starting talos (1.0)

is there any chance that browser_path config entry will have something trailing the process name?

/foo/bar/org.mozilla.fennec/
(Assignee)

Comment 8

8 years ago
for android, browser_path=org.mozilla.fennec.  For possible other platforms (like desktop fennec or n900), it will be /root/path/subpath/fennec.xyz/fennec.  In this scenario this code works as well.

There could be a platform in the future that causes us grief.  And it could be far enough in the future that we would forget about this and have to spend a couple hours debugging.  

The good news is this code is pretty much identical to code that is in devicemanager to detect the process name for processExists().  So if there is something quirky in there other things will fail right away!
(Reporter)

Updated

8 years ago
Attachment #509864 - Flags: feedback?(bear) → feedback+
(Reporter)

Updated

8 years ago
Attachment #509972 - Flags: review?(bear) → review+

Comment 9

8 years ago
Comment on attachment 509864 [details] [diff] [review]
kill fennec before starting mochitest/reftest (1.0)

This looks good.  I thought putting this in remoteAutomation would make more sense, but looking at your patch here, I like that it only hunts and stops fennec when we are first starting the tests, so there is no chance it will disguise any hangs, which could happen if we put it in remoteAutomation.  Bravo.
Attachment #509864 - Flags: review?(ctalbert) → review+

Comment 10

8 years ago
Comment on attachment 509972 [details] [diff] [review]
kill fennec before starting talos (1.0)

While this is in desktop talos code, it's gated on browser_config having remote set, so I think it's pretty safe.  

However, mobile.config does not set the remote: argument in its sample config file.  And remote.config sets the remote attribute to False.

Shouldn't you add changes to this patch to:
* set remote: True in mobile.config
* set remote: True in remote.config

r+ with those changes or some explanation as to why they are safe the way they are.
Attachment #509972 - Flags: feedback?(ctalbert) → feedback-

Comment 11

8 years ago
(In reply to comment #10)
> Shouldn't you add changes to this patch to:
> * set remote: True in mobile.config

mobile.config is currently used for the N900s, which aren't currently utilizing SUT.
(Assignee)

Comment 12

8 years ago
what aki said:)

for remote.config (intended for android), we set remote=True if the ip address is set.  Basically throughout all of the talos code we hing the remote code paths on the remote==True condition.  Here is where we set that:
http://hg.mozilla.org/build/talos/file/2198b71577c3/remotePerfConfigurator.py#l10

If you are using an android system, you have to set '-e org.mozilla.fennec'.  For everything but android, we have to verify that the value specified to -e is valid whether it is local or via a SUT agent.  The point being that you will not be successful with {remote}PerfConfigurator.py unless you are properly specifying remote parameters.

Comment 13

8 years ago
Ok cool, thanks for explaining that.  r+ for my part FWIW
(Assignee)

Comment 15

8 years ago
we are fixing this as part of 634691
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 634691
(Assignee)

Comment 16

8 years ago
wrong bug, we still need to land the m-c patch.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 17

8 years ago
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=8d295a60c458
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
Product: Testing → Testing
You need to log in before you can comment on or make changes to this bug.