Re: Properly binding C libraries using callbacks

[Sam Roberts <vieuxtech@...>]

On Thu, Oct 20, 2011 at 2:07 PM, Rob Hoelz <rob@hoelz.ro> wrote:

In:

> static int l_setcompletion(lua_State *L)
> {
>    luaL_checktype(L, 1, LUA_TFUNCTION);
>
>    lua_pushvalue(L, 1);
>    completionFuncRef = luaL_ref(L, LUA_REGISTRYINDEX);
>    linenoiseSetCompletionCallback(completionCallbackWrapper);

Don't do this:

>    completionState = L;

As soon as  l_setcompletion() returns, L should be treated as
undefined (because it could become so, as you demonstrated).

In ln.linenoise() -- you don't show the implementation but I guess it
looks like below --  you should do this:

void lua_linenoise(lua_State* L) {
   completionState = L; /* L is guaranteed to be defined and valid
until lua_linenoise() returns, so use it */

   char* s = linenoise_do_your_thing(); /* this is the whatever
function will call completionCallbackWrapper() */
   lua_pushstring(L, s);
   return 1;
}

That way the completionCallbackWrapper will use the correct lua_State.

However, it calls lua_pcall(), and that lua code can call
ln.linenoise()... is linenoise reentrant? Can you call back into it
from it's callback? Probably not, and you might want to protect
against that happening:

void lua_linenoise(lua_State* L) {
   if(completionState) {
      luaL_error(L, "linenoise is not reentrant, naughty!");
      }
   completionState = L; /* L is guaranteed to be defined and valid
until lua_linenoise() returns, so use it */
   char* s = linenoise_do_your_thing(); /* this is the whatever
function will call completionCallbackWrapper() */
   completionState = NULL;
   lua_pushstring(L, s);
   return 1;
}

If it is reentrant, and you want to support that, and the
completionCallbackWrapper() can be called multiple times before
ln.linenoise() returns (all things I don't know), then after calling
lua_pcall() in the completionCallbackWrapper, you have to again set
the completionState back to the L that it saved, because that's the
valid one.

> Can someone give me some pointers on the proper way to bind a library that uses callbacks, especially one that doesn't
> allow you to thread your own userdata through the callbacks?  Coroutines make it tricky, and my understanding is that
> storing things in global variables in the binding are a faux pas.

linenoise itself uses global state, it looks like, since
linenoiseSetCompletionCallback()  doesn't take
any kind of context as an argument. In that case, have the
completionState be global doesn't make
the binding any worse than the library it binds to.

Cheers,
Sam