Closed
Bug 968718
Opened 10 years ago
Closed 10 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)
|
4.18 KB,
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•10 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•10 years ago
|
||
Assignee: nobody → hskupin
|
|
Assignee | |
Comment 3•10 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•10 years ago
|
Attachment #8371361 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 4•10 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•10 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•10 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•10 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•10 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•10 years ago
|
||
https://github.com/mozilla/mozbase/commit/5c05d42441a444bba2c2f05969e8e62fd277c591
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•