Closed
Bug 791960
Opened 12 years ago
Closed 10 years ago
Rework Mozmill's sleep/waitFor methods to not spin the event loop
Categories
(Testing Graveyard :: Mozmill, defect)
Testing Graveyard
Mozmill
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: whimboo, Unassigned)
References
Details
Attachments
(1 file)
701 bytes,
application/x-javascript
|
Details |
At the moment bug 789773 is causing us real pain because it end up in an application disconnect failure for Mozmill. Not sure if it is the zombie process which is getting created or something else in handling compartments but at some point the execution stops inside the thread.processNextEvent() call.
To give you some more details on that: In Mozmill we synchronize test steps and therefore we need methods like sleep() or waitFor() which let our code wait until a specified condition has been set. The code looks like:
https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/stdlib/utils.js#L328
function sleep(milliseconds) {
var timeup = false;
hwindow.setTimeout(function () { timeup = true; }, milliseconds);
var thread = Cc["@mozilla.org/thread-manager;1"]
.getService().currentThread;
while (!timeup) {
thread.processNextEvent(true);
}
}
As I have heard from Boris and Gavin a couple of months ago it's probably not a good way to do. So I'm seeking out now for advise how we can make this better.
I hope that with such a change we also have a chance to get the application disconnect sorted out. I would appreciate the feedback from each of you! Thanks!
Comment 1•12 years ago
|
||
It's worth keeping in mind that bug 789773 manifests in a zombie state because our underlying C++ appshell code actually spins the event loop while creating a top-level window: http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsXULWindow.cpp#1776
So the problem there definitely isn't MozMill's fault. That being said, spinning the event loop is a nasty thing to do, especially in JS. Can you rework your synchronization code to use callback functions instead?
Reporter | ||
Comment 2•12 years ago
|
||
This is a simplified testcase which can be used to reproduce the hang in processNextEvent() with Firefox 16.0 Beta 3.
Reporter | ||
Comment 3•12 years ago
|
||
I have talked with Bobby and Justin on IRC and what I got back is that Mozmill should stop spinning the event loop entirely (as already mentioned in the summary) and make use of callbacks instead. But that means that our test code will NO longer by synchronous and introduces all the complexity as what we know from other test frameworks like Mochitest. While this is probably the necessary way to go it will completely change the test framework.
Such a change is not something we can do in a short term. It also would have to be done wisely. So I will put this up as a roundtable item for next weeks Automation Development meeting.
Whiteboard: [mozmill-2.0?][mozmill-1.5-next?]
Reporter | ||
Comment 4•12 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #1)
> It's worth keeping in mind that bug 789773 manifests in a zombie state
> because our underlying C++ appshell code actually spins the event loop while
> creating a top-level window:
> http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsXULWindow.
> cpp#1776
In case of that I will lower the severity of this bug to normal.
Severity: critical → normal
Comment 5•12 years ago
|
||
Maybe generators would do the trick? I think sticking a couple of |yield| statements into the mozmill tests would be a lot less confusing that callbacks. What do you think, Henrik?
Reporter | ||
Comment 6•12 years ago
|
||
Would you mind to give an example? So far I have never used generators and I can't see right now how it would solve the problem. So I would really appreciate to hear more about your idea.
Comment 7•12 years ago
|
||
So, my understand is that mozmill wants to be able to write tests that look procedural even though they're waiting for asynchronous events. So, suppose you have a function like this:
function foo() {
doSomething();
spinTheEventLoopUntilX();
doSomethingElse();
spinTheEventLoopUntilY();
finishUp();
}
you could instead do:
function foo() {
doSomething();
yield function(callback) { addListenerForX(callback); }
doSomethingElse();
yield function(callback) { addListenerForY(callback); }
finishUp();
// Throws StopIteration
return;
}
Then, to invoke foo, you do:
var curr = foo;
function doStep() {
try {
curr()(doStep);
} catch(e) { allDone(); }
}
Just a rough sketch - but does that give you an idea? See https://developer.mozilla.org/en-US/docs/JavaScript/Guide/Iterators_and_Generators for more details
Reporter | ||
Comment 8•12 years ago
|
||
That also looks complicated but most of it could somewhat be hidden in the Mozmill API. I haven't had the time yet to check all this.
Beside that I have found the following post about os.file:
http://dutherenverseauborddelatable.wordpress.com/2012/10/03/asynchronous-file-io-for-the-mozilla-platform/
They are doing file i/o asynchronously but in the example code it looks like they are synchronizing the code again. But same here, I haven't had time to check that.
Reporter | ||
Updated•12 years ago
|
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 9•12 years ago
|
||
Would be something we can do together with the move to Marionette in the not yet specified future.
Whiteboard: [mozmill-future]
Reporter | ||
Comment 10•12 years ago
|
||
Also see task.js as part of the following post:
http://dutherenverseauborddelatable.wordpress.com/2012/10/18/beautiful-off-main-thread-file-io/
Comment 11•12 years ago
|
||
Sorry for bugspam.
Some weeks ago when I did a little googling about yield,
I found a lot of pages, blogs, wikis.
I would like to share them.
Related to workers:
-------------------
- https://javascriptweblog.wordpress.com/2010/06/21/web-workers-vs-the-crazy-flies/
-> http://java-script.limewebs.com/demos/swarm/buzz.html
- http://ejohn.org/blog/web-workers/
- http://xebee.xebia.in/2010/11/02/multithreading-in-javascript-with-web-workers/
Related to yield:
-----------------
- http://blog.monstuff.com/archives/000315.html
- www.neilmix.com/2007/02/07/threading-in-javascript-17/
- http://taskjs.org/
- http://hyperstruct.net/2008/5/27/synchronous-invocation-in-javascript-part-1-problem-and-basic-solution/
- https://github.com/bard/xmpp4moz/blob/4cb392d69ad889337f871d6bfb77d657ccb87076/modules/task.jsm
- http://unixpapa.com/js/sleep.html
Task.js and promises:
---------------------
- https://github.com/jvillalobos/imgur-Command/blob/master/src/bootstrap.js
-> https://github.com/mozilla/gcli/blob/master/docs/writing-commands.md
-> https://bugzilla.mozilla.org/show_bug.cgi?id=708984 Promote Promise.jsm from developer tools to toolkit
--> https://gist.github.com/2763402 Experimenting with promises.
--> https://addons.mozilla.org/en-US/developers/docs/sdk/latest/packages/api-utils/promise.html promise - Addon-SDK
- https://bugzilla.mozilla.org/show_bug.cgi?id=763311 Implement basic "Task.js" interfaces in Toolkit
- https://bugzilla.mozilla.org/show_bug.cgi?id=774816 Make the Promise module available to Toolkit consumers
- http://calculist.org/blog/2011/12/14/why-coroutines-wont-work-on-the-web/
- https://blogs.msdn.com/b/ie/archive/2011/09/11/asynchronous-programming-in-javascript-with-promises.aspx
- https://gist.github.com/3085144 createPromise() -- use promises with libraries that only support callback arguments
Reporter | ||
Updated•11 years ago
|
Whiteboard: [mozmill-future] → [mozmill-2.2?]
Reporter | ||
Comment 12•10 years ago
|
||
Mozmill will reach its end of life soon. We are currently working on getting all the tests for Firefox ported to Marionette. For status updates please see bug 1080766.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Whiteboard: [mozmill-2.2?]
Assignee | ||
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•