CoroutinesFixForModules

Making Hammerspoon and Modules Coroutine Safe

See Problem Description for an overview of what coroutines are and how they are broken in Hammerspoon.

This document focuses on a solution as proposed in https://github.com/Hammerspoon/hammerspoon/pull/2308. In here I describe the changes which were made to how the modules interface with LuaSkin, how it should be done in future modules, how to decide which forms to use and when, and some additional thoughts on how coroutines can be used to make Hammerspoon more responsive.

Note that all of the code examples in this document require Hammerspoon 0.9.79 or newer or a development build of Hammerspoon built from https://github.com/Hammerspoon/hammerspoon/tree/ea693f1d25d3a5e793575ecc68f94452255e89e1 or newer.

If you don't care about the details and just want the bullet points of what to do when modifying existing code, or writing new code, you can jump to TL;DR.

Fixing the Problem

In order to allow LuaSkin to track the current lua_State and use the correct Lua "thread", we have to pass it as an argument to the class. This is done by replacing the class method [LuaSkin shared] with [LuaSkin sharedWithState:(lua_State *)L]. By passing in the state each time we need to use the LuaSkin bridging methods, we ensure that LuaSkin performs it's work on the correct Lua "thread".

For functions and methods added by Hammerspoon modules, the necessary change is usually straightforward. For example, hs.screen.mainScreenwill be changed as follows (see the first part for the original):

static int screen_mainScreen(lua_State* L) {
    LuaSkin *skin = [LuaSkin sharedWithState:L]; // note the use of `sharedWithState:L` instead of just `shared`
    [skin checkArgs:LS_TBREAK];

    new_screen(L, [NSScreen mainScreen]);

    return 1;
}

Only one change is required: LuaSkin *skin = [LuaSkin shared]; became LuaSkin *skin = [LuaSkin sharedWithState:L];. Functions and methods defined in this manner are passed the current Lua "thread" state as the L argument, and we just pass it into LuaSkin. Until sharedWithState: is invoked again, LuaSkin will continue to use the specified state.

While this is sufficient for most functions and methods defined in this manner, it isn't sufficient when Lua callback functions are to be invoked by Objective-C class delegates or macOS events are triggered. For these, we don't have a lua_State variable passed to us. As a special case, we can tell LuaSkin to use the main Lua "thread" state by passing in NULL, for example: LuaSkin *skin = [LuaSkin sharedWithState:NULL];.

This works because macOS events can only be handled when the main application thread is idle. Since the Lua engine is also running on the main application thread, we can be certain that it isn't processing anything on an alternate Lua thread when the delegate or event handler is active -- starting the execution of a Lua code block on the main Lua "thread" is safe at this time because the Lua engine will also be idle.

As an example, the Objective-C code for hs.screen.watcher that triggers a callback when the screen dimensions or number of screens changes will now be as follows:

- (void) screensChanged:(NSNotification*)note {
    if (self.fn != LUA_NOREF) {
        LuaSkin *skin = [LuaSkin sharedWithState:NULL]; // note the use of NULL as the state
        // because `L` wasn't passed into us, we can get it from LuaSkin, and since LuaSkin was told to use the NULL
        // state, we can be assured that this will be the lua_State of the main Lua "thread"
        lua_State *L = skin.L;
        _lua_stackguard_entry(skin.L);
        int argCount = _includeActive ? 1 : 0;

        [skin pushLuaRef:refTable ref:self.fn];
        if (_includeActive) {
            if ([note.name isEqualToString:@"NSWorkspaceActiveDisplayDidChangeNotification"]) {
                lua_pushboolean(L, YES);
            } else {
                lua_pushnil(L);
            }
        }
        [skin protectedCallAndError:@"hs.screen.watcher callback" nargs:argCount nresults:0];
        _lua_stackguard_exit(skin.L);
    }
}

Two other special cases should be noted:

• Whenever an Objective-C code block is scheduled to run asynchronously, if it needs to trigger a Lua callback, the sharedWithState:NULL version should be used for LuaSkin methods within the block, even if the Objective-C block is defined within a function where lua_State was passed in as an argument and can be "inherited" by the block. Because the code within the asynchronous block will not actually run until the Hammerspoon application main thread will be idle, it will need to use the main Lua "thread" just as if it were a delegate method or event callback.

As an example, consider hs.image.imageFromURL:

static int imageFromURL(lua_State *L) {
    LuaSkin *skin = [LuaSkin sharedWithState:L] ; // here we can safely use the lua_State passed into the function
    [skin checkArgs:LS_TSTRING, LS_TFUNCTION|LS_TOPTIONAL, LS_TBREAK] ;
    NSURL *theURL = [NSURL URLWithString:[skin toNSObjectAtIndex:1]] ;
    if (!theURL) {
        lua_pushnil(L);
        return 1;
    }

    if (lua_type(L, 2) != LUA_TFUNCTION) {
        [skin pushNSObject:[[NSImage alloc] initWithContentsOfURL:theURL]] ;
    } else {
        int fnRef = [skin luaRef:refTable atIndex:2];
        [backgroundCallbacks addObject:@(fnRef)] ;

        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void){
            NSImage *image = [[NSImage alloc] initWithContentsOfURL:theURL];

            dispatch_async(dispatch_get_main_queue(), ^(void){
                if ([backgroundCallbacks containsObject:@(fnRef)]) {
                    LuaSkin *bgSkin = [LuaSkin sharedWithState:NULL]; // within the block, however, we have to use the NULL state
                    // and also make sure to use the same `L`, or lua_State, associated with the NULL state as well
                    _lua_stackguard_entry(bgSkin.L);

                    [bgSkin pushLuaRef:refTable ref:fnRef];
                    [bgSkin pushNSObject:image];
                    [bgSkin protectedCallAndTraceback:1 nresults:0];
                    [bgSkin luaUnref:refTable ref:fnRef];

                    _lua_stackguard_exit(bgSkin.L);
                    [backgroundCallbacks removeObject:@(fnRef)] ;
                }
            });
        });
        lua_pushnil(L);
    }

    return 1 ;
}

• If a Lua function or block is invoked with lua_call or lua_pcall, it is possible that the Lua code executed may have resumed a coroutine which may in turn use one or more of our updated Hammerspoon functions or methods. Because of this, when the lua_(p)call function returns, LuaSkin may have a different active lua_State instance value then it did when it was invoked. To correct for this, you should reset the LuaSkin state upon return. You can avoid this entirely by using [skin protectedCallAndTraceback:(int)nargs nresults:(int)nresults], which handles this reset for you automatically, instead of invoking lua_(p)call directly. As most of the Hammerspoon modules do use this LuaSkin method, the following example is contrived, but shows what you would need to do if you wanted to use lua_pcall instead.

static int someFunction(lua_State *L) {
    LuaSkin *skin = [LuaSkin sharedWithState:L] ;
    ... do whatever ...
    lua_pushcfunction(L, someOtherFunction) ;
    int status = lua_pcall(L, 1, 0, 0) ;
    skin = [LuaSkin sharedWithState:L] ; // the lua_pcall may have changed the stored state, so reset it back to *our* state
    // note that we reset the LuaSkin state *before* checking the error state; that way, we can use skin inside the error
    // handler if necessary
    if (status != LUA_OK) {
        ... the error will be the topmost item on the stack, so report or act on it as needed ...
        lua_pop(L, 1) ;
    }
    return 0 ;
}

Again, you can avoid this entirely by using [skin protectedCallAndTraceback:(int)nargs nresults:(int)nresults] instead of lua_(p)call.

TL;DR, or Just Tell Me What To Do

In summary, if you keep in mind the following, it should address 99% of the changes you need to keep in mind:

1. If your C or Objective-C function is passed in a lua_State variable as an argument, make sure to pass this to LuaSkin before using any LuaSkin method with LuaSkin *skin = [LuaSkin sharedWithState:L]. This should take care of almost all functions and methods added by Hammerspoon modules, as well as most of the helper functions used within the modules.

2. If your C or Objective-C function or method is invoked by an event occuring outside of the Hammerspoon application, first make sure that you are on the main application thread (for most delegates and events, this is a given, but check Apple's developer docs because this isn't always the case) and then, use the form LuaSkin *skin = [LuaSkin sharedWithState:NULL] ; lua_State *L = skin.L ;. In most cases, you will know that you need to use this form, because L won't already be defined in the current code context.

3. If you schedule code to be run on the main thread asynchronously, make sure to use the LuaSkin *skin = [LuaSkin sharedWithState:NULL] ; lua_State *L = skin.L ; form within the asynchronously invoked code block, even if skin and L might already be defined outside of the asynchronous block and could be retained within the block's closure.

4. If you use lua_(p)call directly, make sure to re-invoked sharedWithState:L immediately after the lua_(p)call returns. Or avoid this all together and just use [skin protectedCallAndTraceback:(int)nargs nresults:(int)nresults].

Making Hammerspoon more Responsive with Coroutines

Coroutines by themselves are still only executed when the Lua engine is processing something -- it has to encounter coroutine.resume or coroutine.yield to adjust which lua_State is currently active, and as long as the Lua engine is processing code, the Hammerspoon application is not idle. To truly make Hammerspoon responsive during long running code blocks, we need to actually pause the Lua engine so the Hammerspoon application main loop can process queued events.

To facilitate this, your code should use hs.coroutineApplicationYield or it's synonym coroutine.applicationYield to yield from the coroutine. Unlike coroutine.yield, which will just pause execution of the current coroutine, coroutine.applicationYield also schedules a timer to fire almost immediately which will resume the coroutine -- you don't need to write your own watcher or code to resume the coroutine. As a fully functional example, see:

local runYouCleverBoy = true -- our escape clause
-- how to trigger our escape clause
local stopTheWorld
stopTheWorld = hs.hotkey.bind({}, "escape", nil, function()
    runYouCleverBoy = false
    stopTheWorld:disable()
end)

local whoWantsToLiveForever = coroutine.wrap(function()
    local cv = hs.canvas.new{ x = 100, y = 100, h = 100, w = 100 }:show()
    cv[#cv + 1] = { type = "rectangle", fillColor = { white = .1 } }
    cv[#cv + 1] = { type = "text", text = "**", textSize = 75 }
    print(cv)
    while runYouCleverBoy do
      cv[2].text = os.date("%S")
      coroutine.applicationYield()
    end
    cv:delete()
end)

-- start the task that wants to live forever
whoWantsToLiveForever()

While this is running, a canvas will show the seconds portion of the current time until you press the escape key. In the mean time, other events (key bindings, eventtaps, timers, even the Hammerspoon console) are all active.

It should be noted that we have completely ignored passing data back and forth between coroutines and the outside as arguments to coroutine.resume and coroutine.yield. An examaniation of this is beyond the scope of this document; while this feature can be powerful in certain contexts, it isn't relevant to my primary purpose at the moment which is to provide tools which can make Hammerspoon more responsive. I bring it up here only to note that if you do require this functionality of coroutines, you will need to investigate writing your own equivalent to coroutine.applicationYield, as the current implementation does not allow for passing values into or out of the coroutine, other than through global variables.

Additional Implementation Notes

This section is included only as a note in case the idea of trying to make Lua within Hammerspoon truly multi-threaded comes up. The approach taken here of (re)setting the lua_State used by LuaSkin each time sharedWithState: is invoked works only because the Lua engine can only process one Lua "thread" at a time from begining to end of the current Lua code block. As the Lua engine is the only true "user" of LuaSkin, the lua_State value stored in LuaSkin can only change under very specific circumstances and never (if we've coded the modules properly) when the Lua engine is working on a different thread. If we introduce multiple application threads utilizing the shared instance of LuaSkin into the picture, this model completely breaks.

Consider this yet one more reason that LuaSkin is not (appilcation) thread safe. To make it so would require a fundamental rewrite that will likely require a much more significant rewrite of all of the modules than this change requires.

---

Standard disclaimers apply, this document is comprised soley of my own thoughts and interpretations and does not necessarily reflect the beliefs, understanding, knowledge, etc. of anyone else who has, ever will, or ever will not contribute to the Hammerspoon project.

No warranties implied or expressed, etc. etc. etc.