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

Reasoning about multi-threaded code #1687

Open
SquidDev opened this issue Jan 18, 2024 · 0 comments
Open

Reasoning about multi-threaded code #1687

SquidDev opened this issue Jan 18, 2024 · 0 comments
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). area-Minecraft This affects CC's Minecraft-specific content. enhancement An extension of a feature or a new feature.

Comments

@SquidDev
Copy link
Member

One of the easiest mistakes to make with peripherals (in both CC:T and peripheral mods) is accidentally accessing Minecraft state from the computer thread, causing hard to debug race conditions.

While most of the time this can be solved with mainThread = true, there are some cases where things get more complicated, disk drives especially are a mess of atomics, callbacks, and defensive copying of item stacks so we can access it from the computer thread.

While this is never going to be a solved problem, I do think there is room for some tooling to make some of this code easier to reason about.

Documentation

While some javadoc mentions which thread methods are called from, it's not very explicit and obvious.

We should add an annotation to the public API which defines from which thread(s) a method is called.

enum ExecutionThread { COMPUTER, MAIN }
public @interface CalledFrom { ExecutionThread[] value(); }

All interface methods in the public API should be implemented with this method, to make it obvious what can and can't be called safely.

We could possibly be smarter here and take inspiration from the checker framework's @GuiEffect, though my inclination for now is to keep things as simple as possible.

Type checking

Now that we have these annotations, we should build some sort of tool to actually check that our code doesn't access invalid state.

I think there's two options here:

  • Require annotations on every method, then implement something using CheckerFramework/ErrorProne. This would probably be quite simple to prototype (we could probably repurpose @GuiEffect), but I'm wary this will be too explicit/verbose.
  • Global type inference: Rather than adding a compiler plugin, perform static analysis on the class files. This would allow us to perform global type inference instead, which might be nice, or might just be confusing.

Further work

There's almost definitely some more work which we could beyond this. My biggest hurdle with some of this code is working out what threads "own" what data, and thus where it's safe to mutate it. I don't think we're ever going to reach something like Rust's ownership model, but something better than @GuardedBy would be nice.

@SquidDev SquidDev added enhancement An extension of a feature or a new feature. area-Core This affects CC's core (the Lua runtime, APIs, computer internals). area-Minecraft This affects CC's Minecraft-specific content. labels Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). area-Minecraft This affects CC's Minecraft-specific content. enhancement An extension of a feature or a new feature.
Projects
None yet
Development

No branches or pull requests

1 participant