RFC: function coroutine.finally(thread, callback)#187
RFC: function coroutine.finally(thread, callback)#187jkelaty-rbx wants to merge 2 commits intoluau-lang:masterfrom
coroutine.finally(thread, callback)#187Conversation
coroutine.finally(thread, callback)
| The callback is called with two arguments `(ok, err)`: | ||
|
|
||
| - **Normal return:** `callback(true, nil)`. The coroutine's function returned. Return values are not forwarded to the callback; they are available to the caller through `coroutine.resume`'s own return values. | ||
| - **Unhandled error:** `callback(false, err)`. The coroutine terminated with an error. The error value is passed as the second argument **when available.** | ||
| - **External close:** `callback(false, "coroutine was closed")`. `coroutine.close` was called on the coroutine. The coroutine did not complete its work. A default error message is provided so callbacks can identify cancellation without external bookkeeping. |
There was a problem hiding this comment.
Would it be possible to instead return a status string detailing if the thread finished normally/failed/was cancelled, instead of a boolean? Or is this information lost after the thread is dead?
Because right now it's essentially impossible to differentiate between a closed thread and a thread that errored with the message "coroutine was closed".
|
|
||
| ### Already-dead coroutines | ||
|
|
||
| If `coroutine.finally` is called on a coroutine that is already dead, the callback fires immediately on the thread that called `coroutine.finally`. The callback receives `(true, nil)` if the coroutine finished normally, or `(false, nil)` if it errored or was closed. The original error value is no longer available because the coroutine has already been reset. |
There was a problem hiding this comment.
It might be better to just error if the thread is already dead, since this behavior might lead to subtle bugs where you didn't expect the callback to run on the same thread.
You can still get this functionality back by checking the coroutine status and running the function inline, after all.
|
|
||
| ### Multiple callbacks | ||
|
|
||
| Multiple callbacks can be registered on the same coroutine. They are called in LIFO order (last registered, first called). Each callback is implicitly called via `pcall`; if a callback errors, the error is discarded and the remaining callbacks still execute. |
There was a problem hiding this comment.
Can the callbacks yield? I assume not since other callbacks have to also run synchronously.
|
Overall I find this well written and a useful language level feature. However, silencing errors would be a deal breaker for me because it hinders not only debugging but also the detection of issues. For example, if most finally callbacks perform clean up actions then their failure may not introduce logical errors but rather memory leaks; which would be challenging to identify then trace back to the callback. Propagating the error to the caller of resume or close would be preferable, which can be pcalled at that point if desired. Although an alternative of replacing the status and message provided to proceeding callbackss could be a consideration. This would be similar to xpcall recalling the handler with its own error if the handler errors. But this would remove the ability to inspect the callstack through a custom xpcall error handler. |
|
|
||
| The callback is called with two arguments `(ok, err)`: | ||
|
|
||
| - **Normal return:** `callback(true, nil)`. The coroutine's function returned. Return values are not forwarded to the callback; they are available to the caller through `coroutine.resume`'s own return values. |
There was a problem hiding this comment.
If finally provided the values that are still on the stack of the given thread it would allow you to implement await.
local function await(thread)
local result = nil
coroutine.finally(thread, function (...)
result = table.pack(...)
end)
if not result then
-- yield until the other thread has finished
local current = coroutine.running()
coroutine.finally(thread, function ()
coroutine.resume(current)
end)
coroutine.yield()
end
local success = result[1]
if not success then
error(result[2])
end
return table.unpack(result, 2)
endI don't think this needs to be perfect - the guarantee can be "whatever is left on the stack". A library can handle cases where the stack is later cleared by caching the result.
|
|
||
| ### Callback invocation | ||
|
|
||
| The callback is called with two arguments `(ok, err)`: |
There was a problem hiding this comment.
Is the callback allowed to yield? My initial reaction is no and we could always allow it later if we think it's okay.
|
i think it'd be more ideal instead of returning a boolean, it'd return a string literal between "error", "finished", "cancelled" |
Rendered