Closed
Bug 968718
Opened 11 years ago
Closed 11 years ago
[mozprocess] Calling wait() twice after kill() returns 0 instead of -sig
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
Right now we fail in our mozrunner thread tests for stopping the application because the returncode is 0 instead of -9. This happens because in kill() we declare the returncode as 0, and if process is already dead because another thread stopped it, it overwrites the former -sig returncode.
https://github.com/mozilla/mozbase/blob/master/mozprocess/mozprocess/processhandler.py#L114
If the returncode has been set, we should really take that one.
Assignee | ||
Comment 1•11 years ago
|
||
Actually this happens when you call wait() twice after the process has been stopped via kill(). I will have a fix for this today.
Status: NEW → ASSIGNED
Summary: [mozprocess] Process.kill() returns 0 instead of -sig if no process has been killed → [mozprocess] Calling wait() twice after kill() returns 0 instead of -sig
Assignee | ||
Comment 2•11 years ago
|
||
Assignee: nobody → hskupin
Assignee | ||
Comment 3•11 years ago
|
||
Simple patch with an additional test for this problem. It needs to be tested on OS X first before I want to ask for review.
Assignee | ||
Updated•11 years ago
|
Attachment #8371361 -
Attachment is obsolete: true
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8371460 [details] [diff] [review]
Patch v1
Review of attachment 8371460 [details] [diff] [review]:
-----------------------------------------------------------------
Tests are all green on OS X and Linux. Sadly tests are still all skipped on Windows.
Attachment #8371460 -
Flags: review?(wlachance)
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 8371460 [details] [diff] [review]
Patch v1
Review of attachment 8371460 [details] [diff] [review]:
-----------------------------------------------------------------
Looks like William and Andrew are out the whole week. Jonathan, would you mind to review? Also adding Joel in case he could do it.
Attachment #8371460 -
Flags: review?(wlachance)
Attachment #8371460 -
Flags: review?(jgriffin)
Attachment #8371460 -
Flags: feedback?(jmaher)
Comment 6•11 years ago
|
||
Comment on attachment 8371460 [details] [diff] [review]
Patch v1
Review of attachment 8371460 [details] [diff] [review]:
-----------------------------------------------------------------
::: mozprocess/tests/test_mozprocess_wait.py
@@ +104,5 @@
> + "process_waittimeout_python.ini"],
> + cwd=here)
> + p.run()
> + p.kill()
> + p.wait()
should you verify the returncode from the first wait?
Attachment #8371460 -
Flags: feedback?(jmaher) → feedback+
Comment 7•11 years ago
|
||
Comment on attachment 8371460 [details] [diff] [review]
Patch v1
Review of attachment 8371460 [details] [diff] [review]:
-----------------------------------------------------------------
Ditto jmaher's comments.
Attachment #8371460 -
Flags: review?(jgriffin) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Ups, you both are right. I missed to add this. Fixed it now. I will land it tomorrow with fresh eyes. Thanks!
Attachment #8371460 -
Attachment is obsolete: true
Attachment #8371709 -
Flags: review+
Assignee | ||
Comment 9•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•