Make all C -> Lua calls protected and allow user error handlers in uv.run()

[creationix]

This task will fix #99.

The idea is to protect all calls into Lua from C using pcall. If there is an error, it will be handled by the current global error handlers. If there is no current error handler then we'll throw an unprotected lua error (most likely this will crash the process).

Since calls to uv.run() are blocking, then "nested" calls will be all "stacked" in the current C stack. Therefore users can have nested error handlers. A unit test framework may wish to handle all global errors, but only for the duration of it's test unit. It can call and block on uv.run() with a custom error handlers that pertains to that one test. When the test finishes and uv.run() returns, then the old error handler will be restored.

The new signature will be uv.run(mode, onError).

[rphillips]

Perhaps make the errors a ring. Just to make sure it isn't unbounded.

nm... I was confused.

[creationix]

That's not a problem, errors thrown by error handlers won't feed back into themselves.

[creationix]

My implementation idea is to add a couple global variables per luv instance. One will hold a reference to the current error handler function. The other will be a temporary place to hold the error value when an error happens.

The global error handler on the C side will simply store the error in the global place and unblock uv_run() with uv_stop(). Then the luv_run() wrapper will check the global error slot for a possible error.

[creationix]

WIP at #129

[creationix]

I decided this is a very deep rabbit-hole with little benifit. I instead made a simple change to lua_pcall all callback event sources and hard-error if there is an uncaught exception. All existing luv, luvi, and luvit unit tests still pass with this change since we almost always wrap our code in xpcall to get stack traces.

This change will simply enforce the current de-facto behavior and let you know if you forget to protect your event sources.

[creationix]

Implemented in 7526c1d

[neoxic]

Probably should be reopened if you allow my humble five cents...

Catching runtime exceptions isn't the only reason why a C->Lua call should be protected. Studying your code, I can see that you pay no attention to the fact that many Lua C API functions may trigger out of memory exceptions. For example, lua_pushcfunction() that you use in luv_call_callback is prone to exceptions in Lua 5.1 due to the absence of light C functions (it always create a closure). In other words, any function that leads to memory allocation may fail (new strings, tables, string conversions, etc). One must pay attention to the types of exceptions a C API function generates as denoted in the Lua C API manual to the right of their names. What if one wants to confine memory usage of a Lua state and provides a custom lua_Alloc with some sort of accounting when luv is used as a library? Or is it impossible to create a custom Lua state?

Speaking more broadly, a pattern to reliably call a callback must be:

1. lua_cpcall() into a callback wrapper with an argument pointing to a structure on the C stack which you fill with the outer callback's arguments;
2. Inside the wrapper, it is safe to manipulate the stack, so fetch your Lua function, prepare its arguments and call it via lua_pcall() or even lua_call() since you are already protected;

Another problem to this is that in your code, you mix malloc() and the allocator of the Lua state (indirectly). Moreover, many calls to malloc() don't even bother to check if the allocation succeeded or just assert it's successful. The assumption that if a memory block cannot be allocated, the process must terminate is a brave one.:-) I'd recommend allocating internal structures via lua_newuserdata() for protection and consistency or at least use lua_getallocf() to fetch the current Lua state's allocator.

Loosely connected to the above, I can see that luv_alloc_cb() always allocates suggested_size which is always 64Kb by default. This is very slow and ineffective on most systems. I'd recommend buffer pooling.

[creationix]

The assumption that if a memory block cannot be allocated, the process must terminate is a brave one.

I think of it as a non-feature to keep things simpler and easier to maintain. Is this constraint actually a problem for anyone? Does it prevent you from doing something you want to do?

That said, you bring up good suggestions. I'm just not sure who has the bandwidth to implement the change.