[cmd3] Allow triggers to be garbage collected when their creation scope exits - #9310
[cmd3] Allow triggers to be garbage collected when their creation scope exits#9310SamCarlberg wants to merge 1 commit into
Conversation
github-actions
Bot
added
type: testing
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d2ea13355
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd33beca4d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f7f3e45 to
db0b3e3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7f3e459a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db0b3e31af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92acbfc964
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b54ff44c67
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44c3db0d39
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
44c3db0 to
e7dd15d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7dd15d39c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28eddfd3e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
096bd76 to
2c5d239
Compare
SamCarlberg
changed the title
2c5d239 to
7b8790c
Compare
If a trigger is created in a non-global scope S1, cached, and later reused in a different scope S2 (which may be global), any bindings created in S2 would have been immediately cleaned up by the scheduler, effectively making that cached trigger only usable in that original scope S1. Allow EventLoop to be queried for action presence. Avoids issues where custom event loops could be cleared but the triggers still think they're bound. Triggers can now query their event loops directly Add WeakLinkedhashSet to prevent EventLoop from blocking GC. Otherwise, event loop bindings would always be held by the event loop without being able to be garbage collected. For example, triggers created in exited scopes with no references other than their `m_eventLoopCallback` would not be GC'd because the callback is in the event loop. If the loop holds weak references to bindings, the callback won't keep its trigger in a zombie state. The scheduler keeps strong references to triggers as long as their creation scope is active, preventing early GCs when triggers are used inline eg `new Trigger(() -> foo).onTrue(...)`. Note that this has no effect on cached trigger objects like `CommandXboxController.a()` because those caches already have strong references tied to the lifetime of the cache. Once the creation scope exits, the scheduler drops the strong references and allows the triggers to be garbage collected
7b8790c to
e2e56c8
Compare
1 participant
A LinkedHashSet with weak references as entries mean the elements in the set aren't prevented from being garbage collected if they're otherwise unreachable. This fixes the issue of triggers potentially piling up if they're created in nonglobal scopes and aren't cached outside those scopes. However, zombie triggers will continue to be polled until they're garbage collected, potentially incurring minor runtime costs - but the original intent of trigger cleanup was to avoid memory leaks, which weak references is a better solution for.