feat: remove alive flag in session/undeclarable objects
#1104
Conversation
This flag was just used to prevent double close/undeclaration, i.e. object being drop after `close`/`undeclare`. Using `ManuallyDrop` instead in `close`/`undeclare` solve this issue, and save some space in structs (often one word counting the padding, which is not negligible).
|
In general the approach LGTM. However, could you please provide additional comments in the code on how those |
Not sure to have understood, but I've added comments; is it better now?
A lot of tests are already calling EDIT: Actually, the CI caught a bug in my initial |
| @@ -824,16 +827,25 @@ impl Session { | |||
| } | |||
| } | |||
|
|
|||
| /// Like a [`Session`], but not closed on drop | |||
| pub(crate) struct SessionClone(ManuallyDrop<Session>); | |||
There was a problem hiding this comment.
It's unclear to me the need of such SessionClone. What real problem is it solved here?
Since SessionClone contains clones of Runtime, State, etc. how the state is cleaned up?
How is this change robust to internal changes, e.g. Runtime will required to be properly cleaned? If that happens, it seems we will be leaking memory or not freeing resources.
There was a problem hiding this comment.
Yes, that was a mistake of mine.
|
Closed in favor of #1106 |
This flag was just used to prevent double close/undeclaration, i.e. object being drop after
close/undeclare. UsingManuallyDropinstead inclose/undeclaresolve this issue, and save some space in structs (often one word counting the padding, which is not negligible).