georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
137 stars 44 forks source link

`Send` and `Sync` #141

Open kellerkindt opened 1 year ago

kellerkindt commented 1 year ago

What is the status about Send and Sync? There is no explicit unimpl note, but due to the raw-pointers, it is also not auto-impled on Proj. Is it safe to move the Proj instance between threads and have shared calls to .project?

If so, what about providing the impl:

unsafe impl Sync for Proj {}
unsafe impl Send for Proj {}

If not, what about clarifying it in the docs / README.

Background: I would like to load and parse the proj-definition once and call the projection from different places in the application.

urschrei commented 1 year ago

Context creation is cheap and e.g. creating one per thread using Pro::new mirrors the underlying libproj ProjCtx, which is intended to be created per thread.

I'm not sure whether it would be safe to pass the wrapped raw pointer around, but it's unlikely that we'll try to do so without some evidence of a performance concern – the string slice used to create the context can be created immutably and shared / moved without concern, which provides a guarantee that instances created using it will perform identically.

kellerkindt commented 1 year ago

Yeah, it just makes storing it in an otherwise Send + Sync (and readonly) struct really hard (impossible?) or it needs to be instantiated before every use.

frewsxcv commented 1 year ago

My understanding, and I very well might be wrong, is that so long as we're always fetching the error from the context (and not the global context), then it should be Send, I think?

frewsxcv commented 1 year ago

Opened a PR for Send https://github.com/georust/proj/pull/147

kellerkindt commented 1 year ago

This issue might be impossible to implement due to how the errors are retrieved (potential conditions on Sync) and the context being bound to the current thread at creation time (while Send / moving between threads can occur at any time)...?

TomFryersMidsummer commented 3 months ago

Having Send would make writing asynchronous code with Proj much easier.

Could PJ_CONTEXT pointers be moved out of the Proj and ProjBuilder structs into thread-local variables? It would seem to fit well with the advice in the Proj documentation:

It is recommended to create one threading context per thread used by the program.