you might be able to support that through task.desync and task.sync?
Actually yeah, you’re probably right at the moment, I could use those.
But I’d rather wait until parallel Luau has had more time to mature lest simply calling desynchronize isn’t sufficient anymore or something like that.
The Connect function seems to resume on a seperate thread than it was fired in, so that when the thread is suspended by Roblox internally (like when a script is destroyed), the function continues to run. How can you turn this behavior into a function?
The code below is an awful and inefficient wrapper for your code which achieves the behavior, but it is obviously not practical
--// Variables
local signal = require(game.ReplicatedStorage.modules.signal)
local mainSignal = signal.new()
--// Module
local lockedThread = {}
mainSignal:Connect(function(func: any, ...)
func(...)
end)
function lockedThread.spawn(func: any, ...)
mainSignal:Fire(func, ...)
end
-- Creates a new signal when called, and waits for a resulting thread object from task.delay
function lockedThread.delay(duration: number, func: any, ...)
local delaySignal = signal.new()
local indices = 0
local result = nil
-- Create connection and immediately fire signal
delaySignal:Once(function(...)
result = task.delay(duration, func, ...)
end)
delaySignal:Fire(...)
-- Wait for result, then clean up
while not result and indices < 6 do
print("waiting for thread result")
indices += 1
task.wait()
end
if not result then
print("created unsafe thread")
result = task.delay(duration, func, ...)
end
delaySignal:DisconnectAll() -- probably redundant
return result
end
return lockedThread
How can I create a function which works like this, without the awful inefficiency?
Edit: So, the observed behavior only happens in some scenarios? I’m terribly confused and have been losing sleep over this one. Is there even a way to stop a thread from being cancelled when inside a script that is destroyed? You know this stuff better than I do.
Unfortunately it’s not possible to make it immediately stop calling the handler when the original connector goes away, as Lua isn’t privy to the information about exactly when threads get uninitialized on the engine side.
However it is possible to make it stop calling the handler in the exact same way as the RBXScriptSignal but slightly later, the next time a garbage collection cycle happens. Here’s the simplest implementation:
function Signal:ConnectWeak(fn)
local weakConnecterHolder = setmetatable({coroutine.running()}, {__mode = "v"})
local connection;
connection = self:Connect(function(...)
if weakConnecterHolder[1] then
fn(...)
else
connection:Disconnect()
end
end)
return connection
end
1 Like
Bummer. Eventually I tried my method with bindables, didn’t work either. Was slightly surprised by that though; wouldn’t you expect for a method within an instance to run in a different thread/environment than the script it was called in? My guess is that it’s because the Connect function is a method of an RBXScriptSignal which is probably a form of userdata
It’s a bit difficult to understand this part, what’s a use case for ConnectWeak? I’m not familiar with weak references. To my knowledge, it’s creating a weak table for the running thread (co.running()) and if the reference is broken (through gc? not too sure), the connection gets disconnected. When is a weak reference broken? Thanks
Ah, there are actually two behaviors at play here, I think I misunderstood which you were talking about.
-
Built in RBXScriptSignals have a behavior where they disconnect a handler function when the thread which called connect to that handler gets terminated.
-
Any threads you create remember their original source, and when a script gets de-parented it terminates the entire tree of threads spawned out from its original thread. This includes event handlers. Unfortunately it doesn’t actually terminate the coroutines, just the C++ side threads, so there’s no way to detect whether a given coroutine has been terminated in that way, the Lua side threads are still in a suspended state so other code can run them again.
So, it’s not really about being the same or different thread. Because the event handlers always run in a different thread than the caller, even for built in RBXScriptSignals, it’s rather the special behavior linking threads to the thread that spawned them coming into play.
I’m not aware of any way to detect script-deparented termination of the threads which is what would let us handle this. The best you can do right now as far as I know is this:
function Signal:ConnectWithContext(fn)
local localFreeRunnerThread = nil
local function localAcquireRunnerThreadAndCallEventHandler(...)
local acquiredRunnerThread = localFreeRunnerThread
localFreeRunnerThread = nil
fn(...)
localFreeRunnerThread = acquiredRunnerThread
end
local function localRunEventHandlerInFreeThread()
while true do
localAcquireRunnerThreadAndCallEventHandler(coroutine.yield())
end
end
-- Need the coroutine.wrap so that the new runner threads are made in the context
-- of the original connecting thread and terminated along with it.
local makeRunnerThreadFn = coroutine.wrap(function()
while true do
localFreeRunnerThread = coroutine.create(localRunEventHandlerInFreeThread)
coroutine.resume(localFreeRunnerThread)
coroutine.yield()
end
end)
return self:Connect(function(...)
if not localFreeRunnerThread then
makeRunnerThreadFn()
end
task.spawn(localFreeRunnerThread, ...)
end)
end
This will terminate any currently running event handlers when the connecting script gets deparented, however it won’t stop subsequent event handlers from getting run afterwards. We’d need the aforementioned way to detect when a thread had been terminated thanks to script deparenting to make it 100% work.
1 Like
No worries. Just for good measure, let me reiterate with an example
— A Part in the Workspace
— A Script in the Part
— Part is interacted with by a character (touch, click, etc)
(no yield) — Do something to said character after a delay (via task.delay)
— Destroy Part
— Suspended delay thread never resumes now
Obviously with this example, the best solution is just modularization. But the scope of the issue becomes much larger when you add layers.
— Part in Workspace
— Script in Part
— Part interacts with character
— Delay via a timer class
| — Create timer
| — Store timer in table
| (no yield) — Delay for any number duration (via task.delay) to clean it up & remove it from table
— Destroy Part
— Memory leak, as task.delay cannot resume and remove it from table
There is no way to properly secure the timer module other than by using costly loops every heartbeat. The only other way to avoid the issue is to use a fully modularized framework (or specifically modularize scripts which call this function, but that’s very inconsistent), which is definitely not for everyone. Even then, how can you guarantee that there’re no sneaky edge cases?
Anyway, thank you a lot for the help. I’m not sure what I’m going to do just yet but I’ll figure it out eventually
Hi, love your work. I’m having an issue with the linked implementation of GoodSignal:
Calling :Connect on a newly instanced Signal object throws the error ReplicatedStorage.Signal:111: Attempt to get Signal::_handlerListHead (not a valid member).
The referenced lines of the current implementation are:
-- Make signal strict
setmetatable(Signal, {
__index = function(tb, key)
error(("Attempt to get Signal::%s (not a valid member)"):format(tostring(key)), 2)
end,
__newindex = function(tb, key, value)
error(("Attempt to set Signal::%s (not a valid member)"):format(tostring(key)), 2)
end
})
It assumes that any attempts to set or get keys in the Signal object with a corresponding nil value is invalid, but Signal.__handlerListHead is meant to have nil as a valid value so in some cases __index/__newindex is called even though a valid key was index.
Here's a workaround:
-- Make signal strict
setmetatable(Signal, {
__index = function(tb, key)
if key == "_handlerListHead" then return end
error(("Attempt to get Signal::%s (not a valid member)"):format(tostring(key)), 2)
end,
__newindex = function(tb, key, value)
if key == "_handlerListHead" then return end
error(("Attempt to set Signal::%s (not a valid member)"):format(tostring(key)), 2)
end
})
Did you modify your implementation in some way?
If you look at the code _handlerListHead / _next are set to false instead of nil when no value is present for exactly that reason (and because it’s more efficient for the number of members in a table to not change over its lifetime through things getting changed from nil to non-nil).
1 Like
Ah, yeah I must have because I can’t get it to break again 
Sorry about that
Could you possibly add
:Defer() and :Delay()
I made a PR to add ConnectParallel. The method I am using should work since ConnectParallel is just using Connect with task.desynchronize
But, heres a type you can use in your open-source packages:
type ScriptConnection = {
Disconnect: (self: ScriptConnection) -> ();
Connected: boolean
}
type ScriptSignal<T...> = {
Connect: (self: ScriptSignal<T...>, func: (T...) -> ()) -> (ScriptConnection);
Wait: (self: ScriptSignal<T...>) -> (T...);
Once: (self: ScriptSignal<T...>, func: (T...) -> ()) -> (ScriptConnection);
ConnectParallel: (self: ScriptSignal<T...>, func: (T...) -> ()) -> (ScriptConnection);
}
Example Usage:
local MySignal = Signal.new() :: ScriptSignal<...>
Just remember to add parallel support along with changing connected from private to public.
2 Likes
I’m using this library from traditionally using the BindableEvent system, works like a charm, but there’s a weird thing I’d love to know what’s going on…
In this example, the code awaits for a Promise to complete, it does this by waiting for the PlayerDataLoaded GoodSignal event to fire with some business logic present. This code snippet works completely fine.
(the connection variable is stated and initialized in two lines)
return Promise.new(function(resolve)
local connection
connection = PlayerDataService.Events.PlayerDataLoaded:Connect(function(loadedPlayer)
if player == loadedPlayer then
connection:Disconnect()
return resolve(true)
end
end)
end):await()
However, when changing the code to this:
(the connection variable is stated and initialized on one line)
return Promise.new(function(resolve)
local connection = PlayerDataService.Events.PlayerDataLoaded:Connect(function(loadedPlayer)
if player == loadedPlayer then
connection:Disconnect()
return resolve(true)
end
end)
end):await()
I get an attempt to index nil with ‘Disconnect’ error when calling connection:Disconnect(). What’s the issue with assigning the GoodScript Connection object on just one line?
The same reason why you can’t do this:
local Table = {
Value = Table
}
The variable is still being initialized, so it’s not possible to use it inside itself unless it has already been defined above.
1 Like
I’ve not changed anything in my code when suddenly this begun arbitrarily throwing errors: C stack errors and “Error occurred, no output from Lua” errors. Any help would be greatly appreciated as it very randomly can break entire servers: not being able to load data, previous default Roblox chat not loading, etc. It has happened once every day for the past three days, whereas before it would not happen at all.
The following image shows my Health script also erroring after using GoodSignal, despite me not changing it for months.
For now I’ve wrapped Line 139 in a pcall (admittedly I have no clue whether it’ll fix it or not), but any explanations would be great.
Hey @stravant, this doesn’t happen anymore but I still do get C stack overflow errors for unknown reasons. Guidance would be helpful, I’ve tried everything, I’ve used debug.traceback to try to trace where the stack begins but as I don’t know the complex mechanics of the module, I don’t know what exactly is causing this; nested connections? Nested :Fire()? Thanks.
Yes, you have event re-entrancy somewhere.
If you have no idea where to look and don’t have event re-entrancy somewhere else you could add
self._firing = true, self._firing = false around the body of the :Fire call and error if it’s already being fired to find whose fault the re-entrancy is.
Hey, thanks for your response. It appears that the errors are completely anonymous. Have no clue what to do at this point; the error doesn’t even appear after key game events or triggers so I’m completely lost. Why is Roblox not supplying context with the error message?
Note: I can only reproduce this in-game. Haven’t seen it once in studio; perhaps more common when large amount of players in-game?
function Signal.new()
return setmetatable({
_handlerListHead = false,
_firing = false,
}, Signal)
end
function Signal:Fire(...)
local item = self._handlerListHead
if self._firing then
error("re-entrancy")
end
self._firing = true
while item do
if item._connected then
--local trace = debug.traceback()
--local split = trace:split("\n")
--local new = {}
--for _, v in split do
-- local newSplit = v:split(".")
-- table.insert(new, newSplit[#newSplit])
--end
--print(table.concat(new, ", "))
if not freeRunnerThread then
freeRunnerThread = coroutine.create(runEventHandlerInFreeThread)
-- Get the freeRunnerThread to the first yield
coroutine.resume(freeRunnerThread)
end
task.spawn(freeRunnerThread, item._fn, ...)
end
item = item._next
end
self._firing = false
end
Do you have deferred events enabled?
Hey, I use Good Signal.
I’m curious about the relevance of enabling deferred events when Good Signal is written in pure Lua. Typically, the “deferred events” setting only affects Instances.