Stack overflow with detector

[fluxionary]

stacktrace.txt

How to replicate:

  TT
CPDT

C = chest
P = pusher
D = detector
T = tube

The tubes connect the output side of the detector to one of its "inaccessible" sides.

My splint to fix this on Blocky Survival is to prevent any machine from pushing to itself, but this doesn't feel like the "correct" solution. There's some instances where it seems like a machine pushing to itself should be allowed, e.g. distributors and HP pushing chests, though i can't think of any actual use for such a configuration..

[fluxionary]

Actually, nevermind that fix. That fixed my local "replication" of the issue, but the issue on the server seems to involve multiple detectors in a sequence or something, so they're still creating an infinite loop and crashing.

[fluxionary]

And actually, you can create this issue just by creating a long enough linear sequence of detectors (though you need quite a few). No loop required.

[joe7575]

What about that proposal?

diff --git a/tubelib_addons1/detector.lua b/tubelib_addons1/detector.lua
index f7c5ad9..7bfd552 100644
--- a/tubelib_addons1/detector.lua
+++ b/tubelib_addons1/detector.lua
@@ -19,14 +19,17 @@ local function switch_on(pos)
 		node.name = "tubelib_addons1:detector_active"
 		minetest.swap_node(pos, node)
 		minetest.get_node_timer(pos):start(1)
-		local meta = minetest.get_meta(pos)
-		local own_num = meta:get_string("own_num")
-		local numbers = meta:get_string("numbers")
-		local placer_name = meta:get_string("placer_name")
-		tubelib.send_message(numbers, placer_name, nil, "on", own_num)
 	end
 end
 
+local function send_on_msg(pos)
+	local meta = minetest.get_meta(pos)
+	local own_num = meta:get_string("own_num")
+	local numbers = meta:get_string("numbers")
+	local placer_name = meta:get_string("placer_name")
+	tubelib.send_message(numbers, placer_name, nil, "on", own_num)
+end
+
 local function switch_off(pos)
 	if tubelib.data_not_corrupted(pos) then
 		local node = minetest.get_node(pos)
@@ -128,12 +131,12 @@ minetest.register_craft({
 	},
 })
 
-
-tubelib.register_node("tubelib_addons1:detector", {"tubelib_addons1:detector_active"}, {
+tubelib.register_node("tubelib_addons1:detector", {}, {
 	on_push_item = function(pos, side, item)
 		local player_name = minetest.get_meta(pos):get_string("player_name")
+		switch_on(pos)
 		if tubelib.push_items(pos, "R", item, player_name) then
-			switch_on(pos)
+			send_on_msg(pos)
 			return true
 		end
 		return false

It will not solve the "endless line of detectors", but is this a real server scenario?

[fluxionary]

If I'm reading that right, as soon as something possibly comes in, it switches the machine to the "on" node, and so, because you removed the active node from the declaration, no longer can process input. Then it does the check of where to sends stuff, and if that succeeds, it triggers the send message logic.

That sounds like it should work, but here's my 2nd solution on BlS, which simply refuses all input on the detector except from the appropriate side. I reverted the "self loop" check I had earlier. This prevented the issue in all configurations I could cook up.

I did a quick check and am (nearly) certain there aren't any other machines that accept items and immediately send them out again, so this is probably the only change needed to prevent stack overflows (unless something really insane happens). At this point, I feel the issue of someone putting several thousand detectors in series isn't really a sensible concern, so I'm happy with my solution.

The "broken" machines on BlS turned out to be two detectors that were both aimed at each others output, due to a couple of misconfigured teleporters.

[joe7575]

You are right, your proposal is much better. I will take it over. Thanks