Skip to content

[addons] Implement new init function argument dispatch

In Short:

Stop discarding the (addon) init function signature. Instead capture it at compile-time and provide a suitable adapter, thus restoring type safety.

Doing so uncovered a bug-to-happen: The old addon_register_func declared the module argument with type Handle<Value> instead of Handle<Object>. This only compiled because of the discarded type information. It only worked because it exploited numerous v8 implementation details.

It also exposed a few diverged prototypes of Initialize(...) functions. Both is fixed in this patch.

The new init function handling makes a few things easier:

  • Builtin modules had an unused module argument which is no longer neccessary.
  • We no longer have to deal with context aware modules seperatly. It's all the same.

The Issue:

This is not easy to spot and everything works just fine. So let me explain. The addon register function is currently declared as:

typedef void (*addon_register_func)(
    v8::Handle<v8::Object> exports,
    v8::Handle<v8::Value> module,
    void* priv);

But the documentation and thus everybody uses:

void Init(Local<Object> exports, Local<Object> module) {}

Ignore the Handle<> to Local<> thing for now and focus on the switch from Value to Object on the module argument. That looks like an attempted upcast. Now, how does this even compile? It compiles because of this cast in the NODE_MODULE_X(...) macro:

#define NODE_MODULE_X(modname, regfunc, priv, flags)                  \
  extern "C" {                                                        \
    static node::node_module _module =                                \
    {                                                                 \
      /* things */                                                    \
      (node::addon_register_func) (regfunc),    /* <=== EVIL */       \ 
      /* more things */                                               \
    };                                                                \
    NODE_C_CTOR(_register_ ## modname) {                              \
      node_module_register(&_module);                                 \
    }                                                                 \
  }

The intention here is to allow addons to only mention the first few arguments and ignore the rest. Kind of like default arguments but reverse. The result is that we miss the type mismatch and bypass any possible argument conversion that might help. (No such conversion exists, but we miss that too).

It also ignores that in fact Handle<X> and Handle<Y> are two completely unrelated types. Since the handles are passed by value, we not only depend on Handle<X> and Handle<Y> having the same API (which is reasonable), but also on things like sizeof() being equal. We treat them like actual pointers while they are objects passed on the stack. (Again, even if they where pointers such a cast Value to Object would be illegal... without RTTI blah blah).

All of this only works because we know how the handles are implemented and how polymorphism works in v8. Although non of this is likely to change I still think we are way to deep in v8 implementation details.

A minor nuisance is that NODE_MODULE(...) accepts any garbage as a function pointer.

So, we are looking for a way to deal with different init(...) signatures without producing a gaping hole in the type system.

A Solution:

Instead of discarding the type information we use it to "generate" an adapter function at compile-time. This adapter function always takes the full set of arguments and calls the user provided init function with the "requested" subset. This is implemented using template techniques to introspect the init(...) functions at compile-time. This is implemented in the first commit. The lot of it is in node.h and is best read from bottom to top.

The second commit touches all builtin modules and uses the new infrastructure.

All of this is pretty complex. I hope I caught the essence. If not, please ask.

Merge request reports

Loading