Closed Bug 958897 Opened 6 years ago Closed 6 years ago
ssltunnel lives if mochitest killed
After killing `mach mochitest-chrom` with SIGINT (^C) an ssltunnel process is left around. Arch Linux x86_64 Firefox git gecko-dev 985f454a9ca1a5f8c5e4feaa644df2d4893786fc
Whiteboard: [good first bug][mentor=jmaher][lang=python]
How to get started with this ? I would like to contribute
Hi Sayan, here are a few steps to get started: 1) setup your machine to build firefox, get the source and build: https://developer.mozilla.org/en-US/docs/Developer_Guide/Build_Instructions 2) once done, make sure you can run some mochitests (https://developer.mozilla.org/en-US/docs/Mochitest), I would use this as a smaller example: ./mach mochitest-plain content/base/test/test_CrossSiteXHR.html 3) now that you can run tests (ignore the results), start running the test and hit CTRL+C, make sure the browser stops and then ensure that the ssltunnel process is terminated as well. please ask if any of those steps are unclear.
Firefox build was successful but I am getting errors while running Mochitest : First it showed that it failed to bind socket Other tests which followed also failed .
glad to hear you could build Firefox and start running mochitests. If you get a failure to bind socket, that is usually the webserver failing which would mean the entire test failed. We have a websocket server and of course ssltunnel. When running tests locally, it is normal for many tests to fail due to the different environment. This bug is about cleaning up processes if we terminate mochitest while running the test. I would recommend: 1) running mochitest (ignore output) 2) verify ssltunnel process is running (and no firewalls are blocking it) 3) ensure tests are running (even if they are failing) 4) terminate (via ctrl+c) the python harness 5) verify ssltunnel is closed if 5 doesn't happen, then we need to handle ctrl+c in the python harness and ensure our exit paths are all accounted found and they all cleanup.
This problem occurs on my machine.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Please see attached patch. It wraps Mochitest.runApp() with a try-finally block to ensure that any created resources are cleaned up no matter what happens inside. Also note that although the patch is quite large the only change other than indent is adding the `try:` and `finally:` lines.
Comment on attachment 8395393 [details] [diff] [review] Patch to clean up Mochitest.runApp when exceptions are thrown. Review of attachment 8395393 [details] [diff] [review]: ----------------------------------------------------------------- this was a great surprise patch. In the future if a bug is assigned to someone, please find another bug to work on. Likewise if you are working on abug, please make sure it is assigned to you!
Attachment #8395393 - Flags: review+
Assignee: vaibhavmagarwal → kevincox
Sorry, I didn't realize the bug was assigned, I hope I didn't stop on any work. Next time I will double check who it is assigned to.
no worries, all is good
Hi Kevin, While inspecting this bug on windows, I have found out that even after this patch, ssltunnel is not closed in all cases. Also xpcshell and websocket server applications are not closed in windows when we press 'Ctrl+C'. You might want to investigate here.
Hi Vaibhav, First of all I don't run windows so I can't test when it keeps running. I suspect that this may be an issue removing the tempfile. Quoting the python2 documentation for os.remove(): > On Windows, attempting to remove a file that is in use causes an exception to be raised. This may show that there are more exception unsafe functions lower down that are holding the file open. And if you exit inside those functions you get this behavior. That theory would explain the intermittentancy of the issue. On that note: > Also xpcshell and websocket server applications are not closed There are likely more similar issues in the code base but I think that they should be opened as different bugs. If you do open different bugs please cc me, I would be glad to fix them. What do you think?
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.