Skip to content

Conversation

imsodin
Copy link
Member

@imsodin imsodin commented Aug 12, 2023

The current life-time handling of an index handler is both convoluted and there's no check that it is stopped when the model cleans up connection resources (including the index handler).

@calmh
Copy link
Member

calmh commented Aug 16, 2023

I have a slightly different take on this. The way the index handler registry takes the parent Supervisor as a parameter and manipulates that is super funky, I think it should rather be a Supervisor and act accordingly, being stopped/removed by the model when the connection closes. I will file a comparison PR for discussion.

@imsodin
Copy link
Member Author

imsodin commented Aug 16, 2023

I think I get what you are proposing - makes sense. I think it should expose a (suture) service though, not a supervisor, as it shouldn't be used as a supervisor (except internally).

@calmh
Copy link
Member

calmh commented Aug 16, 2023

Right, yes.

@imsodin
Copy link
Member Author

imsodin commented Aug 16, 2023

Did you already do/implement it or do you want me to update this PR?

@calmh
Copy link
Member

calmh commented Aug 16, 2023

#9038

…cthing#9021)

The current life-time handling of an index handler is both convoluted
and there's no check that it is stopped when the model cleans up
connection resources (including the index handler).
@imsodin imsodin force-pushed the model/indexhandler-lifetime branch from 9c5753f to 5467d8d Compare August 19, 2023 08:45
@imsodin imsodin closed this Aug 21, 2023
@imsodin imsodin deleted the model/indexhandler-lifetime branch August 21, 2023 16:11
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Aug 20, 2024
@syncthing syncthing locked and limited conversation to collaborators Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants