-
Notifications
You must be signed in to change notification settings - Fork 23
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
Sessions aren't automatically closed on Drop
#495
Comments
See #265 for some history |
Thanks for the response. I've tried to follow through the discussions in the issues/PR you've linked but I'm still not entirely sure what's the state with the TKMMS issue (iqlusioninc/tmkms#37), and I'm probably missing some context beecause I'm not using TKMMS :) Anyway, from what I understand, #273 turned out to be the bugfix for the issue? Does that mean its safe to simply revert #265? If not, what is the intended way to close the session with the current API? |
Sorry, I should've given more background. While it's possible to revert #265 since the actual culprit was fixed in #273, it's lead me to question the wisdom of executing complicated code in drop handlers. A panic in a drop handler is extremely painful to debug, which is why the original code was using I guess we could try reverting it and see if any problems arise. Things seem nicely stable now though which is why I worry a bit about it. |
Hi,
I've noticed that, contrary to documentation,
yubihsm::session::Session
doesn't get closed when dropped.For example:
results in:
Ive also confirmed that implementing a custom
Drop
onSession
whereself.close()
is explicitly called fixes this issue.Would you mind accepting a PR with this fix?
The text was updated successfully, but these errors were encountered: