Lua restore broken for non-trivial functions before trunk r6253 - General Discussion and Assistance - CHDK Forum  

Lua restore broken for non-trivial functions before trunk r6253

  • 1 Replies

Offline reyalp

  • ******
  • 14000
I recently noticed that "restore" in Lua silently ends after running a handful of Lua statements. The exact number appears unpredictable and depends on the code, but more than 10-20 lines will likely fail. I've checked in a minimal fix in trunk r6253

The updated code allows restore to run for up to roughly one second. Note it does not (and cannot) yield, so kbd task hogging CPU could be an issue, and (unlike ubasic) restore itself cannot be interrupted, since keys are only checked when script is yielded.

This means restore functions should still be kept simple, but the new code show allow you to restore any number of propcases and config values and write something to a log file.

In the previous code, functions that yield (sleep, keyboard functions etc) would also immediately and silently end restore. In the new code, they end with the error message "attempt to yield across metamethod/C-call boundary" (I may improve this later) (edit: changed to "ERROR: cannot yield in restore" in trunk r6255)

The documentation ( notes that yielding functions should not be used in Lua restore, but does not specify what happens.

restore is called via script.c gui_script_kbd_process, ultimately from kbd_process.c kbd_process. Since this is in kbd_task, which runs also runs scripts, it can only be called while the Lua thread in the state Lt has yielded, either from the our auto-yield hook or a call like sleep or the keyboard functions.

The previous restore code would call restore using lua_pcall with the yielded Lua state Lt.  As far as I can tell, this is not actually legal: A yielded thread should be resumed with lua_resume. Calling an unrelated function with pcall in a yielded thread does not appear to be defined, but is also not caught by any error checking.

Lua code is ultimately executed by lvm.c luaV_execute. Among other things, this checks whether hooks like our count hook (luascript.c lua_count_hook) need to be run. After a hook runs, it checks if the Lua state is yielded (since hooks are allowed to yield, as ours does), and returns if it is.

Since Lt was *already* yielded when pcall was called, this means the restore code would silently end the next time the hook was called. This is independent of set_yield values: The hook runs every YIELD_CHECK_COUNT (100) VM instructions and then checks the criteria defined by set_yield, but the luaV_execute check happens regardless. So in practice, this means restore would end after at most 100 VM instructions, but AFAIK the initial value of the count would depend on previous execution history.

The return value of pcall was checked, but pcall (unlike resume) does not distinguish between yield and return, so it was treated as success.

The fix runs restore in the "main" Lua state L instead of Lt. This should be valid: Calling a function in a different thread while Lt is yielded is fine, and globals (like restore) are accessible in all states. Any locals defined in scope visible to restore will also be bound when it is defined.

The one second run time limit is implemented with another hook (lua_restore_count_hook), which just calls luaL_error if the time is exceeded. Lua debug hooks are per state (thread), although lua_newthread copies them from the state passed to it.

Other notes:
The above has existed since Lua support was added. The fact that it's silent and restore is usually just a few settings probably kept it from being noticed, though IIRC there have been complaints about restore being flaky.

ubasic restore behaves differently: ubasic normally yields after every line, and interrupting just causes execution to jump to the restore label if defined. So it can run indefinitely and use yielding functions normally. A second interrupt immediately ends the script.

I think it should be fairly straightforward to implement the ubasic style behavior in Lua (by creating a new thread and adjusting lua_script_run), but I wanted a minimally invasive fix to backport to 1.6.

Also for completeness: In both Lua and ubasic, restore is called only when the script is terminated by the exit key (shoot, by default). It is not called on runtime errors or normal exit.

« Last Edit: 20 / August / 2023, 23:22:48 by reyalp »
Don't forget what the H stands for.


Offline reyalp

  • ******
  • 14000
Re: Lua restore broken for non-trivial functions before trunk r6253
« Reply #1 on: 28 / August / 2023, 00:24:30 »
I merged this into 1.6 in r6256
Don't forget what the H stands for.


Related Topics