Allowing session declarations to stay alive for the lifespan of a session. #96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@DariusIMP any chance this PR makes it to the 1.0 release? I am mainly interested because of the removal of finalize() overrides in the declaration classes. Although I am worried because the Subscriber class seems to still have it. I have encountered several JVM crashes caused by a SIGSEGV because I have closed a session without undeclaring subscribers first, resulting in the subscribers attempting to undeclare themselves afterwards when they are being garbage collected. I can also create a separate issue for that JVM crash but it seems that this PR should fix the problem (if all declaration classes get rid of the finalize method), as well as provide an easier-to-use API. Because currently I have to wrap the Session with my own class in order to keep track of the closeables. |
|
@hassomehide Yes, this will be added to the next release |
DariusIMP
force-pushed
the
issue/background-declarations
branch
from
e54d723 to
66b5dd6
Compare
DariusIMP
force-pushed
the
issue/background-declarations
branch
from
66b5dd6 to
808a42e
Compare
…alive for the lifespan of a session. Zenoh declarations ran for as long as the Kotlin variable representing them was kept alive. This meant that whenever the user lost track of the variable, it got garbage collected and undeclared in the process. This behavior seems to be counterintuitive for programmers used to garbage collected languages (see eclipse-zenoh#43). Therefore in this PR we provide the following change: we keep track of session declarations in a list inside the session, allowing users to keep running them despite losing their references. When the session is finalized, the associated declarations are undeclared. In case the user needs to close a declaration earlier, they need to keep the variable in order to undeclare it.
DariusIMP
force-pushed
the
issue/background-declarations
branch
from
808a42e to
7fe1fdf
Compare
…iately after doing `session.close()`
DariusIMP
force-pushed
the
issue/background-declarations
branch
from
f4e7366 to
ca1ea92
Compare
gabrik
approved these changes
Aug 1, 2024
DariusIMP
added a commit
to ZettaScaleLabs/zenoh-kotlin
that referenced
this pull request
Aug 5, 2024
…sion. (eclipse-zenoh#96) * feat(background declarations): Allowing session declarations to stay alive for the lifespan of a session. Zenoh declarations ran for as long as the Kotlin variable representing them was kept alive. This meant that whenever the user lost track of the variable, it got garbage collected and undeclared in the process. This behavior seems to be counterintuitive for programmers used to garbage collected languages (see eclipse-zenoh#43). Therefore in this PR we provide the following change: we keep track of session declarations in a list inside the session, allowing users to keep running them despite losing their references. When the session is finalized, the associated declarations are undeclared. In case the user needs to close a declaration earlier, they need to keep the variable in order to undeclare it. * feat(background declarations): closing all session declarations immediately after doing `session.close()`
DariusIMP
added a commit
to ZettaScaleLabs/zenoh-kotlin
that referenced
this pull request
Aug 5, 2024
…sion. (eclipse-zenoh#96) * feat(background declarations): Allowing session declarations to stay alive for the lifespan of a session. Zenoh declarations ran for as long as the Kotlin variable representing them was kept alive. This meant that whenever the user lost track of the variable, it got garbage collected and undeclared in the process. This behavior seems to be counterintuitive for programmers used to garbage collected languages (see eclipse-zenoh#43). Therefore in this PR we provide the following change: we keep track of session declarations in a list inside the session, allowing users to keep running them despite losing their references. When the session is finalized, the associated declarations are undeclared. In case the user needs to close a declaration earlier, they need to keep the variable in order to undeclare it. * feat(background declarations): closing all session declarations immediately after doing `session.close()`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Zenoh declarations ran for as long as the Kotlin variable representing them was kept alive. This meant that whenever the user lost track of the variable, it got garbage collected and undeclared in the process. This behavior seems to be counterintuitive for programmers used to garbage collected languages (see #43).
Therefore in this PR we provide the following change: we keep track of session declarations in a list inside the session, allowing users to keep running them despite losing their references. When the session is finalized, the associated declarations are undeclared. In case the user needs to close a declaration earlier, they need to keep the variable in order to undeclare it.