Closed Bug 636451 Opened 11 years ago Closed 11 years ago
devicemanager should have the ability to terminate when it can't launch a second process
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.
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 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.
Attachment #514795 - Flags: review?(mcote) → review+
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 on attachment 514807 [details] [diff] [review] fail if we are already running (1.1) Looks good, thanks!
Attachment #514807 - Flags: feedback?(ctalbert) → feedback+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.