WaitForChild should stop if instance is destroyed

At the moment, WaitForChild will continue to yield on an instance, even if the instance is destroyed. Because nothing more will be added to a destroyed instance, it doesn’t make sense for WaitForChild to continue. WaitForChild should stop if the instance it was called upon has been destroyed.


Proof/reproduction of current issue:

local part = Instance.new("Part")

-- Destroy part after 1 second:
delay(1, function() part:Destroy() end)

-- Wait for non-existent instance within the part, w/ 5 second timeout:
local something = part:WaitForChild("Something", 5)

print(something) --> nil (but it took 5 seconds to get here)

Because the part was destroyed after 1 second, WaitForChild should also stop.


This becomes very dangerous if no timeout is set. If the above code had no timeout, it would yield forever. You would get the “Infinite yield possible” warning, but that’s it.


Proposed solution: WaitForChild should terminate if the instance it’s called upon is destroyed.

17 Likes

If I recall, the underlying listener is cleaned up when the instance is garbage collected, as with any event listener. Guess not.

Related: X:WaitForChild(Y) leaks Lua thread if X is destroyed

1 Like

Ah nice, didn’t find that when I was searching

Here’s the thing…

That’s not technically true, you can add more objects to a destroyed object, and WaitForChild will actually work in this case:

> a = Instance.new(“Part”)
> spawn(function() a:WaitForChild(“Part”) print(“Done”) end)
> a:Destroy()
> Instance.new(“Part”).Parent = a
Done

Given the amount of flakey code people write relying on dubious WaitForChild timing, I would not be surprised if people are actually depending on that behavior in live games too, so it’s very much not “free” to change.

4 Likes

I would argue it’s a bug that items can be added to a destroyed instance

I think the reason it was originally developed that way was to limit the degree to which you would have to special case all your code to handle the fact that a destroyed instance might be handed to it. Regardless of the reasoning or whether you think it’s good behavior or not though, any behavior which has existed for that long has has people depending on it can no longer be treated as a “bug” but is an API change.

4 Likes