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

Fix API safety for control-flow awaitables like until when used from tangentially related fibers #78

Open
ImmutableOctet opened this issue Jul 15, 2024 · 0 comments
Assignees
Labels
C++ Script API Good practice Indicates a proposed change that would be seen as good practice Memory Safety module: engine For features that involve the `engine` module. (optional) Refactor Used to flag significant codebase refactors

Comments

@ImmutableOctet
Copy link
Owner

We currently have a safety issue where calls to until and co. from nested lambdas references the outer Script object, rather than the generated ChildScript for a sub-thread.

i.e. for something like:

start_thread([&]() -> ScriptFiber { co_await until<OnButtonPressed>(); print("Unsafe usage of `until`."); });

we reference the parent thread's until member-function, but co_await the result in the sub-thread, making the generated awaitable reference the wrong script object and fiber. This gives unintended behavior as well as hitting a few asserts.

The ideal behavior here would be to have things just work™, but if that ends up being too difficult we could always assert early to prevent unwanted side effects.

My working idea to fix this would be to go with the OS thread's currently running Script instance, rather than using self in until's implementation. This isn't an ideal solution, as self would not actually have its usual meaning, but it would probably fix the issue.

NOTE: It should actually be safe to reference the parent thread from a sub-thread, since the parent should outlive it, but the awaitable's script reference is still an issue here.

@ImmutableOctet ImmutableOctet added Good practice Indicates a proposed change that would be seen as good practice module: engine For features that involve the `engine` module. (optional) Refactor Used to flag significant codebase refactors C++ Script API Memory Safety labels Jul 15, 2024
@ImmutableOctet ImmutableOctet self-assigned this Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ Script API Good practice Indicates a proposed change that would be seen as good practice Memory Safety module: engine For features that involve the `engine` module. (optional) Refactor Used to flag significant codebase refactors
Projects
None yet
Development

No branches or pull requests

1 participant