Reasoning about multi-threaded code
SquidDev opened this issue ยท 0 comments
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.