Fix non-working lua Lock

[dmaluka]

The lock provided to lua as micro.Lock does not really work: an attempt to use it via micro.Lock:Lock() results in an error:

Plugin initlua: init:260: attempt to call a non-function object stack traceback:
	init:260: in main chunk
	[G]: ?

The reason is that the value that is provided to lua is a copy of the mutex, not the mutex itself.

Ref #1539

This PR makes it work, but generally I'm not sure if directly exposing the Go sync.Mutex object to lua is the nicest way to provide this functionality.

[X-Ryl669]

I'm not sure it's useful to have access to this lock. It's only used in shell.Jobs, autosave and infobar/tab bar event handling.

If you hold it (without releasing it), it breaks micro in subtle way. I don't understand what it is supposed to protect here, since when the event handler is running in micro.go:DoEvent(), lua isn't supposed to run here. If it's made to prevent autosaving a buffer while lua is modifying it, then lua should have a buffer.StartModification / EndModification function so it would be more clear.

[dmaluka]

It's only used in shell.Jobs, autosave and infobar/tab bar event handling.

To be precise, it is used with any event handling, not just with infobar and tabbar events. E.g. a bufpane event is first handled by action.Tabs.HandleEvent(), which then propagates the event to HandleEvent() of the corresponding tab, which then propagates it to HandleEvent() of the corresponding bufpane, which finally handles the event. All that is done under the lock.

when the event handler is running in micro.go:DoEvent(), lua isn't supposed to run here.

If we are talking about lua code running asynchronously, spawned by other lua code (e.g. if it is a timer callback passed to time.AfterFunc()), then micro has no control over when this code is running, so it may run simultaneously with DoEvent(). Right?

That said, I agree with your general sentiment: exposing locking primitives is tricky and may lead (will lead) to deadlocks. So it would be better to avoid it if possible. So, if time.AfterFunc is the only possible case of such asynchronous lua code, perhaps we'd better not export time.AfterFunc at all, and instead export a timer API implemented by micro itself (so that micro itself would prevent races by properly serializing timer callback execution with event handling, autosaving etc, instead of leaving this burden to lua)?

[X-Ryl669]

The issue with exposing our own timer api is that we're loosing the advantages of go channel for inter-routine synchronization (unless making a kind of enum to list all possible interaction that Lua could have done and that need to be "reflected" in go's world).

That being said, I'm pretty sure I might be the only one having used AfterFunc in lua (from a whole github code search, I couldn't find any reference) since it's actually unusable without the other PR and no one had complained before.

So if an higher level API were required, it would need some kind of "Action" list that Lua would build inside the asynchronous function and then let micro's go implementation apply the actions when Lua's isn't running anymore.

A bit like this pseudo code:

local function wait(timeout)
  time.Sleep(timeout)
end

local function modifyBuffer(buffer)
  -- Modify buffer somehow
end

local function uiChange()
   -- Change InfoBar
   -- Change current tab, whatever
   -- Redraw ?
end
 
function triggerAsyncFunc(list)
   table.insert(list, wait, 3000)
   table.insert(list, modifyBuffer, micro.CurPane())
   table.insert(list, uiChange)
   micro.AppendAsyncOp(list)
end

In the go code, it should eval all the items in the list/table with the Lua lock taken (either globally while executing the asynchronous operations) or once per function.

[dmaluka]

So if an higher level API were required, it would need some kind of "Action" list that Lua would build inside the asynchronous function and then let micro's go implementation apply the actions when Lua's isn't running anymore.

Why? We could just let micro control when the asynchronous function is executed.

I.e. implement a Lua-exported function with the same semantics as time.AfterFunc but with the timer callback executed by micro itself.

I see 2 possible ways how to implement this function inside micro:

1. The "easier" way: a simple wrapper around time.AfterFunc: micro's internal timer callback takes the Lock and then runs the callback passed from lua.
2. The more elegant and preferable way: micro's internal timer callback pushes the callback passed from lua to a channel of callbacks, and select in DoEvent() receives the callback from this channel and runs it. I.e. the lua's timer callback is executed in the main goroutine, so ulua.Lock is not needed at all.

Am I missing something?

BTW as a bonus, micro could also take care of redrawing the screen after executing the callback, so #2939 would not be needed. With the option 2 (the callback running in DoEvent()) it would actually be ensured automatically, without extra effort.

[X-Ryl669]

The "easier" way: a simple wrapper around time.AfterFunc: micro's internal timer callback takes the Lock and then runs the callback passed from lua.

What happen if the lua code call AfterFunc from the callback itself (like for an animation)? Does this create another go-routine and risk having a deadlock if that routine is executed/scheduled while the current micro's own afterfunc finishes?

The more elegant and preferable way: micro's internal timer callback pushes the callback passed from lua to a channel of callbacks, and select in DoEvent() receives the callback from this channel and runs it. I.e. the lua's timer callback is executed in the main goroutine, so ulua.Lock is not needed at all.

Yes, I agree, that's probably the easiest and safest solution (I wanted to do something like this in my pseudo code, stacking the callback on the Lua side, but it's probably easier to stack them on Go's side).
I'm not proficient in Go enough to move lua's callback around channels, I don't know if it's possible without a context or how the lifetime of the context is managed in that case.

I think it's still a good idea to have Lua's ability to trigger a Redraw. Not all modification would need a screen redraw (think of remote SSH editing, you probably don't want to perform a complete screen redraw for something that didn't cause UI changes).
Thinking about this, if run in the DoEvent() loop, wouldn't calling redraw be useless anyway ?

I mean, if you perform any action in that "thread", it'll act as if it was done like other synchronous lua code (for example, modifying the infobar), so it'll redraw by itself. However, let's say I'm performing a lot of changes in the buffer (like a global search & replace) I don't want to have 1 redraw per change, but a single redraw when the complete operation is performed.

[dmaluka]

What happen if the lua code call AfterFunc from the callback itself (like for an animation)? Does this create another go-routine and risk having a deadlock if that routine is executed/scheduled while the current micro's own afterfunc finishes?

Seems like it won't deadlock, the new goroutine will wait for the old one, but not the other way around.

Thinking about this, if run in the DoEvent() loop, wouldn't calling redraw be useless anyway ?

Yes, exactly, since it redraws on every iteration of the loop anyway. (Which in itself may be redundant in some cases, so we might think about optimizing it if really needed, but let's not overdesign prematurely.)