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

Fix Clone definition for Injector #8194

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Fix Clone definition for Injector #8194

merged 1 commit into from
Sep 7, 2023

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Sep 6, 2023

Injectors detect that a Picker has been shut down / removed by checking the shutdown atomic bool (which is shared with the Picker instance and is set to true on Picker's Drop). Cloning Injector should create more references to the Picker's atomic bool rather than creating new ones. This should fix global search so that the WalkBuilder halts when the picker is dropped.

@the-mikedavis the-mikedavis added C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer. labels Sep 6, 2023
@the-mikedavis the-mikedavis added this to the 23.9 milestone Sep 6, 2023
@the-mikedavis the-mikedavis force-pushed the injector-clone-fix branch 2 times, most recently from bd10259 to 76d7525 Compare September 6, 2023 15:15
Injectors detect that a Picker has been shut down / removed by checking
the shutdown atomic bool (which is shared with the Picker instance and
is set to `true` on Picker's Drop). Cloning Injector should create more
references to the Picker's atomic bool rather than creating new ones.
This should fix global search so that the WalkBuilder halts when the
picker is dropped.
@archseer
Copy link
Member

archseer commented Sep 6, 2023

Note: that field is typoed as "shutown" but we can fix that later

@pascalkuthe
Copy link
Member

pascalkuthe commented Sep 6, 2023

Note: that field is typoed as "shutown" but we can fix that later

yeah I also pointed that out but @the-mikedavis wanted to fix this in the next release cycle in a separate PR (so this hotfix is as small as possible) since we will need to change this field from a bool to a version integer anyway (so that the dynamic picker can spawn background tasks for stuff like global search and automatically canceled outdated tasks automatically)

@archseer archseer merged commit c0fd8bc into master Sep 7, 2023
6 checks passed
@archseer archseer deleted the injector-clone-fix branch September 7, 2023 02:10
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants