Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: assert(not coroutine.running()) in sync API #7

Closed
linrongbin16 opened this issue Jan 21, 2025 · 4 comments
Closed

Bug: assert(not coroutine.running()) in sync API #7

linrongbin16 opened this issue Jan 21, 2025 · 4 comments

Comments

@linrongbin16
Copy link

linrongbin16 commented Jan 21, 2025

Hi,

In current implementation, there is an issue: linrongbin16/gitlinker.nvim#257

@loganswartz found it is always failed on this line (on fedora 41, but worked on MacOS M1 and ubuntu 20.04):

https://github.com/linrongbin16/gitlinker.nvim/blob/df0c4e342742812186f87e32caf82b3e5e288a84/lua/gitlinker/commons/async.lua#L78

Note: it is copied from here:

assert(not coroutine.running())

I found the comments says there is a API break change in coroutine.running() between lua51 and lua52:

async.nvim/lua/async.lua

Lines 16 to 23 in ad097c9

-- Coroutine.running() was changed between Lua 5.1 and 5.2:
-- - 5.1: Returns the running coroutine, or nil when called by the main thread.
-- - 5.2: Returns the running coroutine plus a boolean, true when the running
-- coroutine is the main one.
--
-- For LuaJIT, 5.2 behaviour is enabled with LUAJIT_ENABLE_LUA52COMPAT
--
-- We need to handle both.

In a previous version, there's a M.running() to handle this:

-- Note: coroutine.running() was changed between Lua 5.1 and 5.2:
-- - 5.1: Returns the running coroutine, or nil when called by the main thread.
-- - 5.2: Returns the running coroutine plus a boolean, true when the running
--   coroutine is the main one.
--
-- For LuaJIT, 5.2 behaviour is enabled with LUAJIT_ENABLE_LUA52COMPAT
--
-- We need to handle both.
--- Returns whether the current execution context is async.
---
--- @treturn boolean?
function M.running()
  local current = coroutine.running()
  if current and handles[current] then
    return true
  end
end

And there is a strict parameter to tolerate some errors:

--- Create a function which executes in an async context but
--- called from a non-async context.
--- @tparam function func
--- @tparam boolean strict Error when called in non-async context
function M.void(func, strict)
  vim.validate({ func = { func, "function" } })
  return function(...)
    if M.running() then
      if strict then
        error("This function must run in a non-async context")
      end
      return func(...)
    end
    return M.run(func, nil, ...)
  end
end
@linrongbin16 linrongbin16 changed the title Bug: coroutine.running() API changes between lua51/lua52? Bug: handle coroutine.running() API break changes? Jan 21, 2025
@linrongbin16 linrongbin16 changed the title Bug: handle coroutine.running() API break changes? Bug: assert(not coroutine.running()) in sync Jan 21, 2025
@linrongbin16 linrongbin16 reopened this Feb 1, 2025
@linrongbin16
Copy link
Author

I just reproduced this bug in linrongbin16/gitlinker.nvim#262, I think it worth some attention.

@linrongbin16 linrongbin16 changed the title Bug: assert(not coroutine.running()) in sync Bug: assert(not coroutine.running()) in sync API Feb 1, 2025
@lewis6991
Copy link
Owner

This repo is in the process of a complete rewrite so this issue might not apply after that.

@linrongbin16
Copy link
Author

This repo is in the process of a complete rewrite so this issue might not apply after that.

Thank you for the updates!

@lewis6991
Copy link
Owner

I've merged in all the v2 code now. It's not yet finished, but is much higher quality than what was on main before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants