So today, one of my users in my game did something that completely broke Roblox.
Using F3X building tools, they managed to select a player, and then hit Ctrl + U. This “ungrouped” them, meaning every child in the player is now in the workspace. Instantly, every single player started lagging immensely, and some PC’s crashed.
The issue? A Humanoid parented to the workspace (It has to do with how a humanoid works).
I tested in studio, and it didn’t lag on a baseplate… but as soon as I loaded a larger map, it almost crashed studio before I managed to delete it.
If there is any possibility of this happening in your game, you should add the following script to both the server and client (both a script and localscript) to prevent this bug from freezing the game.
local RS = game:GetService("RunService")
workspace.ChildAdded:Connect(function(child)
if child:IsA("Humanoid") then
warn("HUMANOID IN WORKSPACE")
RS.Stepped:Wait()
child:Destroy()
warn("DESTROYED HUMANOID")
end
end)
It is also worth noting that locking the player’s parts might also prevent this issue, but I believe that every game should just have this implemented anyways.
Proper way to do this now is actually to defer it so it happens reasonably within the same frame or as soon as possible after ChildAdded happens.
workspace.ChildAdded:Connect(function (child)
if child:IsA("Humanoid") then
task.defer(child.Destroy, child)
end
end)
The overall better idea here is to prevent this from happening in the first place instead of applying an after-the-fact solution. You can do this by locking parts on the same level as a Humanoid so that they can’t select and therefore ungroup the model containing the Humanoid. Not sure how hard I would endorse ChildAdded for Workspace.
That’s not true. Using defer will defer it until the next frame. If you want it to happen as soon as ChildAdded is fired then just delete it like in the example given.
You sure? Doesn’t a frame have more than one resumption point? What you’ve said doesn’t coincide with the information on related threads.
What I was told about defer is that it will either “push the action to occur at the end of the current resumption point or the beginning of a new one if we aren’t currently in one”. If you defer an action to happen within an event connection then it’ll happen at the next available resumption point (e.g. if you’re at the Stepped point of a frame then the thread is deferred to Heartbeat, not the next frame).
Assuming you’re using Default (implicitly Immediate) SignalBehaviour you can’t destroy an instance immediately, which then I understand the justification of needing something to push back the time in which the destruction occurs. Defer will make it happen as soon as possible though whenever the next invocation point is available (RenderStepped → wait resumes → Stepped → Heartbeat → BindToClose). RunService events explicitly wait until that point in the frame is reached.
If you’re beyond Stepped and your code is waiting on it to fire then your script is waiting for the next time Stepped is fired which would be the next frame. Defer would have it resume as soon as possible at the next invocation point in the current frame which could either be in the current frame or the next frame. Defer should be the option you use here.
The equivalent of my solution is using Deferred SignalBehaviour and immediately destroying the instance. In Immediate SignalBehaviour the connected function will be spawned when ChildAdded fires. In Deferred signal mode, the connected function will occur at the next best time. Deferring when the ChildAdded handler runs is equivalent to deferring the Destroy call that’d otherwise not be called via the task library in Immediate mode.
@x86_architecture You can use that code when you have Deferred SignalBehaviour. This is what happens when you’re running Default which currently maps out to Immediate SignalBehaviour:
(cc @grilme99 - This is the basis for my response, but I might not know better, which is why I ask if you’re sure that what I said wasn’t true.)
@Abcreator Right: I didn’t say that BindToClose runs every frame, check the context. BindToClose is a possible invocation point for where deferred threads can be resumed at. If BindToClose is called, any deferred threads can be resumed post-call. Source. Full context:
Doesn’t say “BindToClose runs every frame”. I called BindToClose an invocation point, exactly the same way engineers have defined it in the SignalBehaviour thread.
Pretty sure this is not true, all the possible invocation points are from runservice iteself and bindtoclose isn’t a event in runservice so its not a invocation point that defer can use
Right; please check the thread I linked which is hyperlinked under “source”. BindToClose is an invocation point, but AFAIK that invocation point isn’t reached until it’s actually called which is when the server closes down and the server is running closure operations.
Task scheduler resumes are also considered an invocation point and there’s no related RunService event, you slot your actions there with methods Roblox exposes (wait, spawn, delay and the task library). It’s just that those points are convenient enough for the engine to determine “hey, this is a place we can run code at!”. RunService events occur each frame, so reasonably, they’d be great places to define as invocation points to let threads resume.
I’m hardly using my own words. These are things an engineer has written down.
Does this lag occur if the humanoids are all parented to a different instance such as a Folder in workspace, if so this code won’t work in those situations.
My theory here is that since Workspace is technically a sub-class of Model, or the humanoid is a child of workspace, the exposed humanoid treats every part in Workspace as the limb of a character. Some internal code that relies on this logic then runs checks and other things on all those parts every frame.
There was this one announcement not too long ago about humanoids not setting CanCollide every frame, but only when the humanoid’s state changes:
Maybe something similar is happening every frame with other properties or physics-based interactions; the computations are just multiplied by the large number of parts.
You’re correct that what’s going on is that the Humanoid is treating the entire workspace as a character, because the workspace IsA("Model"), and that this has spectacularly bad performance implications.
I’ve actually opened a discussion with the avatar team recently about having the humanoid special case out the Workspace and not treat it as a character despite it being a model to prevent this accidental behavior.
I’m not too familliar with the technical side of it, but basically if you set it immediately it will refuse to do it.
You will end up getting: "Something unexpectedly tried to se the parent of Humanoid to NULL while trying to set the parent of Humanoid. Current Parent is Workspace."
What @colbert2677 said is 100% correct and matches the information found by Roblox staff.
Using task.defer is a much better option and is a much faster solution (assuming the SignalBehaviour is set to Deferred which will be soon the default signal behaviour) rather than destroying an instance after a frame, because task.defer defers it at a slightly later time in the same frame.
-- Assuming deferred signal behaviour which will soon be the default:
local before = os.clock()
task.defer(function()
print(os.clock() - before) --> 0.0087852000142448
end)
print(task.wait()) --> 0.01009729999987
With signal behavior set to immediate, the results are approximately the same. But this doesn’t mean you should not use task.defer, because the signal behavior will soon be deferred by default (and Immediate will no longer be available).
-- Assuming deferred signal behaviour which will soon be the default:
local before = os.clock()
task.defer(function()
print(os.clock() - before) -->0.02603672999987
end)
print(task.wait()) --> 0.02009729999987
Sure, the difference isn’t much noticeable but task.defer has it’s own usecases. One of the main usecase is the destroying of an instance in the same frame (not the next frame), but at a slightly later time.
I’ve actually opened a discussion with the avatar team recently about having the humanoid special case out the Workspace and not treat it as a character despite it being a model to prevent this accidental behavior.
A good solution would be to check if the model belongs to a player, through Players:GetPlayerFromCharacter(model) and treat it as such.
Pretty sure this is not true, all the possible invocation points are from runservice iteself and bindtoclose isn’t a event in runservice so its not a invocation point that defer can use
This is incorrect, bind to close is an invocation point. It’s literally stated by a Roblox engineer in the thread colbert linked.
That unfortunately doesn’t work, because humanoid does some expensive extra rendering and game logic stuff even for non-player characters. We had a brief chat and it seems like special casing out workspace specifically is the best option.
Wow, this should be stated somewhere as I was using Workspace as a storage place for a humanoid that controlled a teams health, if I knew it was bad performance wise, I would have used a folder with-in Workspace.
A better system might be to use ReplicatedStorage or ServerStorage so you can be sure nothing happens with it. Although it might also be better just to use a simple NumberValue, although I am not too sure what your exact use-case is.
I used a humanoid because with it I could treat it like a humanoid (I could use :TakeDamage() and .Died), this can’t be done as easily on a NumberValue.
I used workspace as it was a place all clients could access that Scripts could also run in.
