Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel waiting threads if signal disconnects all connections #164

Merged
merged 7 commits into from
Oct 26, 2023

Conversation

Sleitnick
Copy link
Owner

Closes #163

This also fixes a potential for FireDeferred to invoke disconnected handlers in scenarios in which a handler disconnects another handler.

@Sleitnick
Copy link
Owner Author

Due to potentially breaking changes, this is a major version bump.

For example:

local s = Signal.new()

task.spawn(function()
	print("Wait...")
	local res = s:Wait()
	print("Got res:", res)
end)

s:DisconnectAll()
s:Fire(32)

Previous Behavior

Before this change, DisconnectAll (and also Destroy) would not cancel out threads waiting via the Wait method. Thus, the above code would successfully print "Got res: 32" when the signal is fired, even though DisconnectAll was called.

New Behavior

This PR would prevent the "Got res: 32" output, because DisconnectAll cancels the waiting thread.

@Sleitnick
Copy link
Owner Author

Another behavior change is regarding FireDeferred, which will now cancel its scheduled events if the signal is disconnected before the tasks are fired.

local s = Signal.new()

s:Connect(function(n) print(n) end)

s:FireDeferred(10)
s:DisconnectAll()

Previous Behavior

The connection will still pick up the deferred event, and print 10.

New Behavior

The deferred firing will be cancelled, and nothing will be outputted.

@Sleitnick Sleitnick merged commit 7549a07 into main Oct 26, 2023
2 checks passed
@Sleitnick Sleitnick deleted the bugfix/signal-wait branch October 26, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signal: Wait doesn't get canceled out on DisconnectAll or Destroy
1 participant