Allow working with a non-finalizing Pkcs11

[wiktor-k]

Hi folks,

There is much interest in a Pkcs11 that does not finalize the underlying implementation and I thought I'll try out one idea I had: splitting the Pkcs11 into an owning and ref structs (similarly to how Rust does with String and str). I've made the Pkcs11 finalize itself and the Pkcs11Impl just work on what's passed in. This way, folks that manage the lifecycle themselves can construct a new instance via Pkcs11Impl::new_unchecked which should resolve #208.

I've also dropped inner Arc from Pkcs11 (so that the concurrency is now managed by the client outside of Pkcs11) and made Session own a reference to the client. I'd further rename Pkcs11Impl into something like Pkcs11Ref but I want to keep the diff as small as possible, and in case you don't like it it's not worth the refactor.

See what you think about it @hug-dev and @Jakuje.

The alternative posted on Slack by @testingapisname is adding additional finalization flag: testingapisname@6a4e58d

[Jakuje]

I do not have this use case and I do not feel knowledgeable enough of rust to be able to judge which of the approaches is better. But this one looks more straightforward, while the other more "bolted on top of the existing things".

Generally, I think it is a good idea, especially when we have people asking for this.

[wiktor-k]

Yep, good point. I've pinged folks in #208 to check if this makes sense (I'd rather avoid making changes without backing approval from real-world users).

Thanks for your help!

[hug-dev]

Thanks for adding this!!

The first commit makes sense: better to let the user decide if they want shared ownership with Arc or not! Makes it clearer in the tests using multiple threads as well.

[hug-dev]

I like the idea here! Was only wondering if we could expand the documentation (and maybe change the name?) to make clear that this is done so that finalize is not called on Drop and maybe explain the use-case.

[wiktor-k]

I've added better docs... happy to hear suggestions about the naming though... new_non_finalizing? new_manage_your_own_lifecycle? I'm kind-of bad at naming 😅

[wiktor-k]

I think this is a step in the right direction, although I see follow-up work (just outlining here so I don't forget):

• considering running initialize in Pkcs11::new so the object is always initialized (just like it's always finalized on drop)
• moving functions from Pkcs11 to Pkcs11Impl since currently even with Pkcs11Impl::new_unchecked it's actually not possible to call further functions 😬 (since they are on Pkcs11 and it's not possible to create that one from Pkcs11Impl)... or maybe I forgot something
• Pkcs11::finalize could return an error... that'd give the caller the ability to finalize and get the finalization error... which is currently impossible (the error is only logged)... this means Pkcs11::finalize would need to directly call finalize_ref and suppress running the Drop (not to have finalization running twice).

Phew 😅 hopefully I didn't forget about anything!

[hug-dev]

moving functions from Pkcs11 to Pkcs11Impl since currently even with Pkcs11Impl::new_unchecked it's actually not possible to call further functions 😬 (since they are on Pkcs11 and it's not possible to create that one from Pkcs11Impl)... or maybe I forgot something

ahh that's a good point 😅 Seems like @testingapisname could also be a cheap solution with just a new finalization flag