[Dominik][0.17.28] Rare memory corruption from mod moving fluid entities

[talchas]

Using https://mods.factorio.com/mod/PickerDollies (entity.teleport followed by update calls mostly) to move a particular underground pipe and disconnect from the machine it's attached to is crashing this save and then I think during the symbolized stack tracing free() complains about memory corruption (and then the game hangs rather than exiting cleanly). Usually this works, though if it's always corrupting memory that's hardly good news.

The particular pipe is slightly to the northeast of the player location, the methane one attached to the steam cracker. Moving it north crashes the game after a moment.

A possibly related thing is that moving various entities with a fluidbox using the mod sometimes randomly fails, and tends to keep failing for that particular entity instance, and now that I've looked at the source it might actually be coming from teleport() failing.

(mods at http://talchas.net/bug-report-mods.7z )

[Dominik]

Hi, I don't see any memory corruption but I see that the mod is doing something weird. It first teleports the pipe 20 tiles south and then 21 north to the intended position. And on the south position it connects to some wrong fluid. So I suggest starting from the mod author.

[talchas]

Yeah, the double teleport thing is weird, and may be the actual cause, but some part of it is still crashing the game (and oddly enough sometimes only after like 300ms delay). Is incorrect teleport()s (that do something like causing invalid fluid linkages or whatever) sufficiently weird that it's fine (ie, the bug may just be "teleport onto a position mixing fluids crashes, even without update_connections() (because it isn't for fluids?)")

[Dominik]

Thing is, I am not sure why it crashes, it does not crash for me. I just get couple of yelling about fluid mixing from my debugger and then it returns to a functioning state. And I would much rather research a bug after the mod works reasonably first, the teleporting around just makes things complicated.

[talchas]

Ok, it looks like the likely issue is that a pipe-to-ground being teleport()ed sometimes doesn't always check against if the target location will cause it to link to another pipe-to-ground with a different fluid. Other times it does. Depending on which fluid wins this can wind up with an invalid fluid hooked up. I've currently got a machine with one gas visibly listed in its input but when I attach something I get 0.0 of a different gas listed in the pipe.

I've been playing around with it and haven't been able to figure out consistent behavior of when it is incorrectly permmitted and when it isn't; also I haven't been able to get an actual crash except with that specific movement (including the teleport back), and not on 0.17.31.

[Dominik]

Btw how should it behave? I recall teleport does not do some checks, including the fluid mixing, so it can lead to various errors. So is it even correct to use teleport? Isn't there some safer function available? Sorry, this is not my area, is why I am asking.

[Bilka]

Btw how should it behave? I recall teleport does not do some checks, including the fluid mixing, so it can lead to various errors. So is it even correct to use teleport? Isn't there some safer function available? Sorry, this is not my area, is why I am asking.

Just like inserting fluid into fluidboxes via lua, teleporting entities (via lua) should not lead to fluid mixing. There is no other option to move entities.

[Dominik]

The fluid mixing was mostly treated, but not in all cases, I have fixed that now. But the mod is really doing weird stuff, jumping 15 one way, 20 another...

[talchas]

Yeah, what it's doing is moving the object out of the way to see if it would normally be legal in the target position. teleport() on its own ignores this (except now for fluid mixing), and can_place_entity() would otherwise say false for any non-1x1 entity because it would overlap the current position. Thus teleport "away", check the target position, teleport back. For non-ghost entities temporarily marking it for deconstruction, doing can_place with ghost_place, and then cancelling the deconstruction now works (I think on older versions it probably wouldn't), but that doesn't work for ghosts since marking them for deconstruction destroys them. For my personal copy I just did that and had it not work with ghosts anymore; it might be possible for the mod to do something like make an empty surface it keeps around for this sort of temporary space, or now that teleport is fixed, keep this hack and try different positions until it finds one that works.

[Dominik]

This is not my area but are you sure? Afaik teleport as any other script buildability check should allow an entity to overlap with itself.

[talchas]

Teleport does, the issue is that teleport also has no problem allowing the entity to overlap with other entities, and the mod wants to prevent things like that that you couldn't normally do. It uses surface.can_place_entity to do so, which obviously just thinks the entity you're teleporting is just another normal entity that's blocking the target location.