mt-mods / pipeworks

Pipeworks is a mod for Minetest allowing the crafting and usage of pipes and tubes
Other
14 stars 19 forks source link

race condition with concurrently arriving stacks #120

Open SwissalpS opened 6 months ago

SwissalpS commented 6 months ago

Reported by Huhhila: concurrently-arriving stacks of same type do not respect the do-not-split-stacks setting

https://blockexchange.minetest.ch/schema/Huhhila/allow_splitting_off_not_respected_when_concurrent

so far only ever heard of other people reporting this with centrifuges, but I think every node with the "allow splitting off" setting is affected.

FeXoR-o-Illuria commented 6 months ago

I'll try to describe what I think happens: Send 2 stacks with 98 items (of the same type with a maximum stack size of 99) to arrive simultaneously at a centrifuge set to "Allow splitting stacks from tubes" off. -> 1) Routing of stacks in the last tube segment in front of the centrifuge: Both stacks will be routed to that centrifuge (has higher priority than e.g. an alternative path to another tube) - after all the input slot is empty and any ONE of the stacks would fit in (but not both - which is not checked/stored) 2) Remove stacks in tubes, add items of stacks to inventory ... and send the overdue items as a new stack back in tubes: The centrifuge will take items from the stacks independent of the "Allow splitting stacks from tubes" setting (So to the viewer it looks like if the setting was violated) -> One stack is taken completely, from the other stack one item is taken, the rest (97 items) send back in the tube

I think it's 1) where the setting has an effect (Stacks that won't fit into the centrifuge are rejected to go it's way when they won't fit completely into it). But at that point in time both traversing stacks would fit in so they both are routed to the centrifuge. After that decision the setting has no effect whatsoever - and thus the "perceived" misbehavior.

Fix: If that is correct the only sane way to solve this I can see is to remove the stack from the tube at 1) and add it to the input inventory of the container basically skipping the last path through the tube (And if you must add a dummy stack traversing the last half of the tube to the container visually only [Maybe a 0 size stack? Actually I have no clue :D]).

P.S.: Fix proposal by Huhhila: "that makes me think of not-particularly-simple approaches like adding an incoming-inventory sized to match free input inventory slots with nodetimer heap... eww, that wouldn't be exactly small amount of code. (... and just one nodetimer per node anyway, except such heap could emulate having more ...)"