636451 - devicemanager should have the ability to terminate when it can't launch a second process

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

currently when we are launching a process, we check if it is running first and spit out a warning.  The problem is we continue to run and timeout because our intended process never launched.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fail if we are already running (1.0)

this patch adds in an option to fail a launchProcess/fireProcess call if we detect the process is already running.  Clint pointed out that launchProcess will return None if we fail getDeviceRoot() or if we fail fireProcess().  I think in both of these situations we should expect things to fail miserably.  It is unfortunate that we don't have a error code system, but this seems acceptable to me.

In addition, while testing this on a windows box, I found that we had some backslash issues and a simple fix in pushFile does the trick to get us up and running.

Comment 2

[Mark Côté [:mcote]]

Comment on attachment 514795 [details] [diff] [review]
fail if we are already running (1.0)

+    if (os.name == "nt"):
+      destname = destname.replace('//', '\')

Not sure what this is doing... should this not be .replace('\\', '/')?  It's translating from a Windows path to a Unix path, right?

-    if self.fireProcess(cmdline) is None:
+    if self.fireProcess(cmdline, failIfRunning) is None:

Btw, in general you should be careful with "is".  It verifies that the two objects are one and the same--not just that their *values* are the same.  In this case it's fine, but in general == is preferred unless you have a really good reason to be comparing object identities.

Anyway good aside from my question up top.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fail if we are already running (1.1)

updated for the fix.  I had transposed these and didn't catch it before hand.  I did a quick cleanup of some rouge print statements as well.

Comment 4

[cmtalbert]

Comment on attachment 514807 [details] [diff] [review]
fail if we are already running (1.1)

Looks good, thanks!

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=0c11cf1fe594