clean up timers a bit

src/timers.cc

@@ -4,102 +4,82 @@
 
 using namespace v8;
 
+static Persistent<ObjectTemplate> timer_template;
+
 class Timer {
  public:
-  Timer(Handle<Function> callback, int argc, Handle<Value> argv[], ev_tstamp after, ev_tstamp repeat);
+  Timer(Handle<Function> callback, ev_tstamp after, ev_tstamp repeat);
   ~Timer();
-  Local<External> CreateTimeoutID ();
-  void CallCallback ();
+
+  static Handle<Value> setTimeout (const Arguments& args);
+  static Handle<Value> setInterval (const Arguments& args);
+  static Handle<Value> clearTimeout (const Arguments& args);
+
+  Persistent<Object> handle_;
+
  private:
-  ev_timer watcher;
-  Persistent<External> timeoutID;
-  Persistent<Function> callback;
-  int argc;
-  Persistent<Value> argv[];
+  static Timer* Unwrap (Handle<Object> handle);
+  static void OnTimeout (EV_P_ ev_timer *watcher, int revents);
+  ev_timer watcher_;
 };
 
-static void
-onTimeout (struct ev_loop *loop, ev_timer *watcher, int revents)
+Timer* 
+Timer::Unwrap (Handle<Object> handle)
+{
+  HandleScope scope;
+  Handle<External> field = Handle<External>::Cast(handle->GetInternalField(0));
+  Timer* timer = static_cast<Timer*>(field->Value());
+  return timer;
+}
+
+void
+Timer::OnTimeout (EV_P_ ev_timer *watcher, int revents)
 {
   Timer *timer = static_cast<Timer*>(watcher->data);
 
-  timer->CallCallback();
+  HandleScope scope;
+
+  Local<Value> callback_value = timer->handle_->Get(String::NewSymbol("callback"));
+  if (!callback_value->IsFunction()) 
+    return;
+
+  Local<Function> callback = Local<Function>::Cast(callback_value);
+
+  TryCatch try_catch;
+  callback->Call (Context::GetCurrent()->Global(), 0, NULL);
+  if(try_catch.HasCaught())
+    node_fatal_exception(try_catch);
 
   // use ev_is_active instead?
   if(watcher->repeat == 0.) 
     delete timer;
 }
 
-Timer::Timer (Handle<Function> _callback, int _argc, Handle<Value> _argv[], ev_tstamp after, ev_tstamp repeat)
+Timer::Timer (Handle<Function> callback, ev_tstamp after, ev_tstamp repeat)
 {
   HandleScope scope;
-  callback = Persistent<Function>::New(_callback);
-  argc = _argc;
 
-  ev_timer_init (&watcher, onTimeout, after, repeat);
-  watcher.data = this;
-  ev_timer_start (node_loop(), &watcher);
+  handle_ = Persistent<Object>::New(timer_template->NewInstance());
+  handle_->Set(String::NewSymbol("callback"), callback);
+
+  Local<External> external = External::New(this);
+  handle_->SetInternalField(0, external);
+
+  ev_timer_init(&watcher_, Timer::OnTimeout, after, repeat);
+  watcher_.data = this;
+
+  ev_timer_start(node_loop(), &watcher_);
 }
 
 Timer::~Timer ()
 {
-  ev_timer_stop (node_loop(), &watcher);
-
-  callback.Dispose();
-
-  timeoutID.Dispose();
-  timeoutID.Clear();
+  ev_timer_stop (node_loop(), &watcher_);
+  handle_->SetInternalField(0, Undefined());
+  handle_.Dispose();
 }
 
-void
-Timer::CallCallback ()
-{
-  HandleScope scope;
-
-  TryCatch try_catch;
-
-  callback->Call (Context::GetCurrent()->Global(), argc, argv);
-
-  if(try_catch.HasCaught())
-    node_fatal_exception(try_catch);
-}
-
-Local<External>
-Timer::CreateTimeoutID ()
-{
-  HandleScope scope;
-
-  Local<External> timeoutID_local = External::New(this);
-
-  timeoutID = Persistent<External>::New(timeoutID_local);
-
-  return scope.Close(timeoutID_local);
-}
-
-static Timer *
-UnwrapTimeoutID (Handle<External> timeoutID)
-{
-  HandleScope scope;
-
-  Timer *timer = static_cast<Timer*>(timeoutID->Value());
-
-  return timer;
-}
-
-// timeoutID = setTimeout(func, delay, [param1, param2, ...]);
-// timeoutID = setTimeout(code, delay);
-// 
-// * timeoutID is the ID of the timeout, which can be used with
-//   clearTimeout.
-//
-// * func is the function you want to execute after delay milliseconds.
-//
-// * code in the alternate syntax, is a string of code you want to execute
-//   after delay milliseconds. (not recommended)
-//
-// * delay is the number of milliseconds (thousandths of a second) that the
-//   function call should be delayed by. 
-NODE_METHOD(setTimeout)
+Handle<Value>
+Timer::setTimeout (const Arguments& args)
 {
   if (args.Length() < 2)
     return Undefined();
@@ -113,52 +93,14 @@ NODE_METHOD(setTimeout)
 
   if (args.Length() > 2)
     assert(0 && "extra params to setTimeout not yet implemented.");
-  int argc = 0;
-  Handle<Value> argv[] = {};
-  /*
-  int argc = args.Length() - 2;
-  Handle<Value> argv[] = new Handle<Value>[argc];
-  // the rest of the arguments, if any arg parameters for the callback
-  for(int i = 2; i < args.Length(); i++)
-    argv[i - 2] = args[i];
-  */
 
-  Timer *timer = new Timer(callback, argc, argv, after, 0.0);
+  Timer *timer = new Timer(callback, after, 0.0);
 
-  Local<External> timeoutID = timer->CreateTimeoutID();
-
-  return scope.Close(timeoutID);
+  return scope.Close(timer->handle_);
 }
 
-// clearTimeout(timeoutID)
-NODE_METHOD(clearTimeout)
-{
-  if (args.Length() < 1)
-    return Undefined();
-
-  Handle<External> timeoutID = Handle<External>::Cast(args[0]);
-  Timer *timer = UnwrapTimeoutID(timeoutID);
-
-  delete timer;
-
-  return Undefined();
-}
-
-// intervalID = setInterval(func, delay[, param1, param2, ...]);
-// intervalID = setInterval(code, delay);
-// 
-// where
-// 
-// * intervalID is a unique interval ID you can pass to clearInterval().
-//
-// * func is the function you want to be called repeatedly.
-//
-// * code in the alternate syntax, is a string of code you want to be executed
-//   repeatedly.
-//
-// * delay is the number of milliseconds (thousandths of a second) that the
-//   setInterval() function should wait before each call to func.
-NODE_METHOD(setInterval) 
+Handle<Value>
+Timer::setInterval (const Arguments& args)
 {
   if (args.Length() < 2)
     return Undefined();
@@ -170,16 +112,23 @@ NODE_METHOD(setInterval)
 
   ev_tstamp after = (double)delay / 1000.0;
 
-  if (args.Length() > 2)
-    assert(0 && "extra params to setInterval not yet implemented.");
-  int argc = 0;
-  Handle<Value> argv[] = {};
+  Timer *timer = new Timer(callback, after, after);
 
-  Timer *timer = new Timer(callback, argc, argv, after, after);
+  return scope.Close(timer->handle_);
+}
 
-  Local<External> timeoutID = timer->CreateTimeoutID();
+Handle<Value>
+Timer::clearTimeout (const Arguments& args)
+{
+  if (args.Length() < 1)
+    return Undefined();
 
-  return scope.Close(timeoutID);
+  HandleScope scope;
+  Local<Object> handle = Local<Object>::Cast(args[0]);
+  Timer *timer = Timer::Unwrap(handle);
+  delete timer;
+
+  return Undefined();
 }
 
 void
@@ -187,8 +136,11 @@ NodeInit_timers (Handle<Object> target)
 {
   HandleScope scope;
 
-  NODE_SET_METHOD(target, "setTimeout", setTimeout);
-  NODE_SET_METHOD(target, "clearTimeout", clearTimeout);
-  NODE_SET_METHOD(target, "setInterval", setInterval);
-  NODE_SET_METHOD(target, "clearInterval", clearTimeout);
+  timer_template = Persistent<ObjectTemplate>::New(ObjectTemplate::New());
+  timer_template->SetInternalFieldCount(1);
+
+  NODE_SET_METHOD(target, "setTimeout", Timer::setTimeout);
+  NODE_SET_METHOD(target, "setInterval", Timer::setInterval);
+  NODE_SET_METHOD(target, "clearTimeout", Timer::clearTimeout);
+  NODE_SET_METHOD(target, "clearInterval", Timer::clearTimeout);
 }

test/test-setTimeout.js

@@ -3,10 +3,15 @@ include("mjsunit");
 function on_load () {
   assertInstanceof(setTimeout, Function);
   var starttime = new Date;
+
   setTimeout(function () {
       var endtime = new Date;
       var diff = endtime - starttime;
       if (diff < 0) diff = -diff;
       assertTrue(900 < diff || diff < 1100);
   }, 1000);
+
+  // this timer shouldn't execute
+  var id = setTimeout(function () { assertTrue(false); }, 500);
+  clearTimeout(id);
 }