Skip to content

Add API to cancel async jobs - #14602

Merged
y5nw merged 8 commits into
luanti-org:masterfrom
y5nw:async-canceling
Aug 26, 2025
Merged

y5nw merged 8 commits into
luanti-org:masterfrom
y5nw:async-canceling

Conversation

@y5nw

@y5nw y5nw commented Apr 30, 2024 •

Copy link
Copy Markdown
Contributor

This PR adds an API to cancel async jobs.

To do

This PR is a Work in Progress.

  • Implement replacing jobs (Dropped)
  • Implement job cancellation
  • Add tests for
    • Replacing jobs before the job is run (Dropped)
    • Replacing jobs after the job is complete (Dropped)
    • Canceling jobs
  • Add documentation

How to test

The new API should be covered by unittests.

Note that I have not yet figured out how to test certain things mentioned in the PR, such as canceling existing async jobs on shutdown.

@sfan5 sfan5 added Feature ✨ PRs that add or enhance a feature @ Script API labels May 2, 2024
@Zughy Zughy added the Concept approved Approved by a core dev: PRs welcomed! label May 2, 2024
@y5nw
y5nw marked this pull request as ready for review June 3, 2024 08:38

@sfan5 sfan5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some thoughts

Comment thread doc/lua_api.md Outdated
Comment thread doc/lua_api.md Outdated
Comment thread src/script/cpp_api/s_async.h Outdated
Comment thread src/script/lua_api/l_async.cpp Outdated
@y5nw

y5nw commented Jun 3, 2024

Copy link
Copy Markdown
Contributor Author

I almost forgot: should this feature also be available for the main menu environment?

@y5nw
y5nw force-pushed the async-canceling branch from 1925ae4 to 4e9b4c8 Compare June 3, 2024 12:16
@sfan5

sfan5 commented Jun 3, 2024

Copy link
Copy Markdown
Member

I almost forgot: should this feature also be available for the main menu environment?

No need atm. If someone needs it in the future, they can add it.

Comment thread src/script/cpp_api/s_async.cpp Outdated
Comment thread doc/lua_api.md Outdated
@y5nw y5nw changed the title Add API to replace an existing async job Add API to cancel async jobs Jun 3, 2024
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread src/script/lua_api/l_async.cpp
Comment thread src/script/lua_api/l_async.cpp Outdated
Comment thread src/script/lua_api/l_async.h
Comment thread src/script/scripting_server.h Outdated
@sfan5 sfan5 self-assigned this Jun 4, 2024
Comment thread doc/lua_api.md Outdated
@sfan5
sfan5 self-requested a review June 22, 2024 12:30

@sfan5 sfan5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 if the unit tests pass

Comment thread src/script/cpp_api/s_async.h Outdated
Comment thread src/script/cpp_api/s_async.cpp Outdated
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread src/script/cpp_api/s_async.cpp
@sfan5

sfan5 commented Sep 6, 2024

Copy link
Copy Markdown
Member

@appgurueu maybe you could review this?

Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread doc/lua_api.md
@y5nw
y5nw force-pushed the async-canceling branch 2 times, most recently from 8c9b422 to 3aea108 Compare November 22, 2024 14:45
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
@sfan5
sfan5 self-requested a review December 10, 2024 20:21
@sfan5 sfan5 removed their assignment Dec 23, 2024
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread games/devtest/mods/unittests/async_env.lua
Comment thread games/devtest/mods/unittests/async_env.lua Outdated
Comment thread src/script/lua_api/l_async.cpp Outdated
Comment thread src/script/lua_api/l_async.cpp Outdated
@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Feb 6, 2025
@y5nw y5nw removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Feb 18, 2025
@sfan5
sfan5 requested a review from appgurueu April 8, 2025 20:02
@grorp

grorp commented May 31, 2025

Copy link
Copy Markdown
Member

apparently this should be "Action/change needed": https://irc.luanti.org/luanti-dev/2025-02-18#i_6243730

@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label May 31, 2025
@y5nw
y5nw force-pushed the async-canceling branch from 2fd7386 to 5b25114 Compare May 31, 2025 19:43
@y5nw

y5nw commented May 31, 2025 •

Copy link
Copy Markdown
Contributor Author

I called off merging back then because I realized that the second run of the unittests doesn't actually complete, but it seems unrelated to this issue.

Aside from that the unittests here still pass, so this PR should be ready to be merged unless I overlooked something else.

@y5nw y5nw removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label May 31, 2025
@sfan5

sfan5 commented Aug 25, 2025

Copy link
Copy Markdown
Member

Ping

@nerzhul nerzhul left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good PR thanks

@y5nw
y5nw force-pushed the async-canceling branch from 5b25114 to f009a72 Compare August 26, 2025 09:04
@y5nw
y5nw merged commit f390137 into luanti-org:master Aug 26, 2025
19 checks passed
mjz19910 pushed a commit to mjz19910/luanti that referenced this pull request Aug 29, 2025
* Implement API to cancel async jobs

Co-authored-by: sfan5 <sfan5@live.de>

* update AsyncJob:cancel documentation from review

* Use IPC to unblock async

* review

* review async unblocking

* review

* Apply suggestions from code review

Co-authored-by: sfan5 <sfan5@live.de>

* minor licensing

---------

Co-authored-by: y5nw <y5nw@protonmail.com>
Co-authored-by: sfan5 <sfan5@live.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature @ Script API >= Two approvals ✅ ✅

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants