Add stack to promise.wait().

The problem was that if promise A was waiting and promise B was created and
then also told to wait (from some callback coming off the event loop), and
then promise A finished, promise B's wait would return. Promise A's wait
would not return until promise B was finished. This is incorrect.

To solve this issue properly, one probably needs to allocate separate
execution stacks. I use, instead, Poor Man's Coroutines. We continue to use
the main execution stack and force promises created most recently to return
first.

That is even if Promise A finishes first, neither wait() returns. Not until
Promise B finishes, will its wait() return. After that is complete, Promise
A's wait() will return.

This introduces the problem of growing the "wait stack" infinitely. Thus
I've added a strong warning to the documentation only to use this operation
sparingly. require() and include() seem to be the proper use case for such a
thing: they are called usually at program start up - they don't take too
long to finish and they won't be called so often.

Let's experiment with this stop-gap. If the infinite promise stack becomes a
problem for many, then I will remove promise.wait() entirely or perhaps only
use it for thread pool events.

src/events.cc

@@ -22,6 +22,9 @@ using namespace node;
 
 Persistent<FunctionTemplate> EventEmitter::constructor_template;
 
+/* Poor Man's coroutines */
+static Promise *coroutine_top;
+
 void
 EventEmitter::Initialize (Local<FunctionTemplate> ctemplate)
 {
@@ -33,6 +36,8 @@ EventEmitter::Initialize (Local<FunctionTemplate> ctemplate)
   constructor_template->PrototypeTemplate()->Set(String::NewSymbol("emit"), __emit);
 
   // All other prototype methods are defined in events.js
+
+  coroutine_top = NULL;
 }
 
 static bool
@@ -187,19 +192,38 @@ void
 Promise::Block (void)
 {
   blocking_ = true;
+
+  assert(prev_ == NULL);
+  if (coroutine_top) prev_ = coroutine_top;
+  coroutine_top = this;
+
   ev_loop(EV_DEFAULT_UC_ 0);
+
+  assert(!blocking_);
 }
 
 void
-Promise::Detach (void)
+Promise::Destack ()
 {
-  if (blocking_) {
+  assert(coroutine_top == this);
   ev_unloop(EV_DEFAULT_ EVUNLOOP_ONE);
+  coroutine_top = prev_;
+  prev_ = NULL;
+}
+
+void
+Promise::Detach (void)
+{
+  /* Poor Man's coroutines */
   blocking_ = false;
+  while (coroutine_top && !coroutine_top->blocking_) {
+    coroutine_top->Destack();
   }
+
   if (ref_) {
     ev_unref(EV_DEFAULT_UC);
   }
+
   ObjectWrap::Detach();
 }
 

src/events.h

@@ -46,11 +46,15 @@ class Promise : public EventEmitter {
 
   bool blocking_;
   bool ref_;
+  Promise *prev_; /* for the prev in the Poor Man's coroutine stack */
+
+  void Destack ();
 
   Promise () : EventEmitter()
   {
     blocking_ = false;
     ref_ = false;
+    prev_ = NULL;
   }
 };
 

test/mjsunit/test-wait-ordering.js

@@ -0,0 +1,30 @@
+include("mjsunit.js");
+
+function timer (t) {
+  var promise = new node.Promise();
+  setTimeout(function () {
+    promise.emitSuccess();
+  }, t);
+  return promise;
+}
+
+order = 0;
+var a = new Date();
+function test_timeout_order(delay, desired_order) {
+  timer(0).addCallback(function() {
+    timer(delay).wait()
+    var b = new Date();
+    assertTrue(b - a >= delay);
+    order++;
+    // A stronger assertion would be that the ordering is correct.
+    // With Poor Man's coroutines we cannot guarentee that.
+    // Replacing wait() with actual coroutines would solve that issue.
+    // assertEquals(desired_order, order);
+  });
+}
+test_timeout_order(10000, 6); // Why does this have the proper order??
+test_timeout_order(5000, 5);
+test_timeout_order(4000, 4);
+test_timeout_order(3000, 3);
+test_timeout_order(2000, 2);
+test_timeout_order(1000, 1);

website/api.txt

@@ -173,6 +173,15 @@ If there were multiple arguments to +"success"+ then they are returned as an
 array.
 +
 If +"error"+ was emitted instead, +wait()+ throws an error.
++
+*IMPORTANT* +promise.wait()+ is not a true fiber/coroutine. If any other
+promises are created and made to wait while the first promise waits, the
+first promise's wait will not return until all others return. The benefit of
+this is a simple implementation and the event loop does not get blocked.
+Disadvantage is the possibility of situations where the promise stack grows
+infinitely large because promises keep getting created and keep being told
+to wait(). Use +promise.wait()+ sparingly--probably best used only during
+program setup, not during busy server activity. 
 
 
 === Standard I/O