World thread-safety#507
Conversation
This ensures the world is not accessed from another thread. Closes dan200#410
This means one can call .getFamily() in a thread-safe manner, ensuring turtle.getFuelLimit() does not cause issues. As we use a specialist TE class for each family this does not require any specialist caching.
If it causes a save corruption then, arguably, it is the end of that world. 🤣 I'll show myself off out... |
|
As you may be able to tell, the monitor changes are slightly larger than anticipated. As the potential for breakage is larger, I'd recommend some serious testing before merging. We're currently using this on SwitchCraft, which caught a lot of the issues before they ended up in the PR. If anyone's interested in helping test, please download from the CI server. |
SquidDev
force-pushed
the
feature/world-thread-safety
branch
4 times, most recently
from
62ff0cc to
5e8b4c7
Compare
This restructures monitor in order to make it thread-safe: namely removing any world interaction from the computer thread. Instead of each monitor having their own terminal, resize flag, etc... we use a monitor "multiblock" object. This is constructed on the origin monitor and propagated to other monitors when required. We attempt to construct the multiblock object (and so the corresponding terminal) as lazily as posible. Consequently, we only create the terminal when fetching the peripheral (not when attaching, as that is done on the computer thread). If a monitor is resized (say due to placing/breaking a monitor) then we will invalidate all references to the multiblock object, construct a new one if required, and propagate it to all component monitors. This commit also fixes several instances of glLists not being deleted after use. It is not a comprehensive fix, but that is outside the scope of this commit.
Whilst this may add some performance overhead to terminal interactions, it does ensure resizing terminals does not result in null pointer or out of bounds exceptions.
SquidDev
force-pushed
the
feature/world-thread-safety
branch
2 times, most recently
from
52e5aaa to
97c5a7c
Compare
|
Whilst the last commits aren't strictly world/computer threading issues, but they fix various aspects about monitors and it was easier to bundle them here instead. I'm happy to revert them if inconvenient. There's some other fixes I'd like to add at some point, but I'd like to confirm they work and are needed first. |
Shader mods may perform multiple passes when rendering a tile, so monitors will be drawn transparently on later passes. In order to prevent this we allow drawing the a single tile multiple times in a tick. See dan200#534
Shaders appear to ignore all the other subtle (and not-so-subtle) hints we drop that monitors shouldn't be rendered with shadows. This solution isn't optimal, as monitors may still be tinted due to sunlight, but there is nothing we can do about that. Many thanks to ferreusveritas for their help in diagnosing, fixing and testing this issue. Closes dan200#534
|
Can one of the admins verify this patch? |
|
For what it's worth, I've been running a fork with almost all of my PRs merged on a relatively popular server and not seen any issues (it's actually more stable thanks to this pr). That being said, there's not really much point bumping PRs. Dan merges stuff when he has time. While it's a little infuriating seeing stuff just sit there, I don't think it's fair to keep pestering Dan either. |
|
I don't remember adding this comment... I think my Jenkins server pulled them all and auto-commented. My bad. |
|
Ahhh, that explains a lot. I was rather confused when I woke up this morning and there were ~30 PRs with the same comment! |
3 participants
Whilst the initial purpose of this was to fix world-thread safety, this PR has rather devolved into a general bugfix PR as it's easier than creating several inter-dependent PRs. Sorry.
This is an attempt at ensuring peripherals/other Lua calls do not access the world from the Lua thread. I'm going to be honest, none of the fixes are especially elegant. I'm expecting several of the fixes to require some level of polling, which is a pain. Currently most peripherals implement
ITickableso it's not the end of the world, but it'd be nice to reduce the number which do in the future.Current progress of the PR:
IMediaprovider but I've been unable to actually cause it, so goodness knows.I'd really appreciate any testing people could do, as there may be subtle differences in behaviour. Similarly, if people are aware of any other thread-unsafe accesses, do tell.
It's worth noting that some of these changes will probably become redundant in the 1.13 update as we'll probably want to split peripherals into their own separate classes. I still think it's worth getting this in before that though.