Fix and improve init/destroy & Blosc2Guard

[milesgranger]

Will close #32

[decathorpe]

Thanks for taking a look - I'm not sure that this is enough though.

For example, it is still possible to call blosc2::compress without first having called blosc2::init, from what I can tell.

[milesgranger]

That's not my responsibility as far as I can see. If the user wants to avoid the overhead of synchronization provided by Blosc2Guard or the safe init/destroy then they can use the unsafe alternatives. Point is they need to initialize blosc2 and how they do that is up to them and they have a safe way to do that and an unsafe way.

[decathorpe]

To my knowledge: If you provide a "safe" public API, and that API can be called in a way that causes memory unsafe things to happen, then that is a bug since it violates the invariants that must be upheld if you only use safe code.

[milesgranger]

I'm happy to have a PR, I'm either incapable or not competent enough to fix this it seems.

[decathorpe]

As far as I can tell, exposing the non-context-based C APIs on the Rust side is just inherently unsafe. For example,
there is nothing that would stop a program from calling blosc2::compress in two concurrent threads - causing them to read/write to the same static memory.

I think the only way to make these APIs safe to use on the Rust side would be to wrap them in a Context struct that you can only obtain once and that is neither Send or Sync - which would ensure that init and destroy are called exactly once, and that no other threads could access the same static memory.

But at this point, you'd have basically re-implemented the context-based C API in Rust, so I'm not sure if that would even be worthwhile.

[milesgranger]

How's about a more emphasized warning/disclaimer that this is "safe", so long as one either uses the context API or remembers to handle the initialization and cleanup, either by the provided safe synchronization wrappers or on their own w/ the unsafe direct calls. Would that be an okay compromise?

[decathorpe]

I don't think that's enough. The non-context APIs currently violate invariants that are assumed by the Rust compiler (for example, they make it easy to read / write global memory without synchronization). To my knowledge, this means that it's possible that the Rust compiler will just generate broken programs because it assumes that the invariants are upheld by all safe functions, when they are not.

EDIT: It looks like blosc2_compress and blosc2_decompress do use a pthread_mutex to synchronize some of their accesses to static memory, but not all of it, from what I can tell. There's also other functions like blosc2_{get,set}_nthreads which doesn't synchronize static memory read / writes to g_global_context at all.

[milesgranger]

Sounds like a disclaimer along the lines of "don't use this crate" is needed then. 😉

[decathorpe]

I can prepare an "informational" advisory for the rustsec database, if that helps?