undeflife / libreoffice-rs

Rust binding to LibreOfficeKit
Apache License 2.0
15 stars 7 forks source link

Clone on Office #15

Closed inzanez closed 2 years ago

inzanez commented 2 years ago

Hi there

quick question: do you need Office to implement Clone? If it weren't, one could share it between threads in a Mutex. As it is now that does not work.

I could PR otherwise,...

undeflife commented 2 years ago

https://github.com/undeflife/libreoffice-rs/blob/master/src/lib.rs#L289 Clone is for sharing reference with callback blocks

quick question: do you need Office to implement Clone? If it weren't, one could share it between threads in a Mutex. As it is now that does not work.

I am not quite clear about does not work, could you please provide more details?

inzanez commented 2 years ago

@undeflife I tried to integrate a running libreoffice instance into a web service using Rocket. To share the instance among threads one would use Managed State. But this has Send and Sync as a requirement.

I did use an Arc<Mutex<Office>> to try and share the office instance but it seems that it requires to be Send, which it cannot be while implementing Clone. The error I get is:

error[E0277]: `*mut _LibreOfficeKitClass` cannot be sent between threads safely
   --> test/src/main.rs:31:14
    |
31  |     office: &State<Arc<Mutex<Office>>>,
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^ `*mut _LibreOfficeKitClass` cannot be sent between threads safely
    |
    = help: within `Office`, the trait `std::marker::Send` is not implemented for `*mut _LibreOfficeKitClass`
    = note: required because it appears within the type `Office`
    = note: required because of the requirements on the impl of `Sync` for `std::sync::Mutex<Office>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `Arc<std::sync::Mutex<Office>>`
note: required by a bound in `rocket::State`
   --> /home/test/.cargo/registry/src/github.com-1ecc6299db9ec823/rocket-0.5.0-rc.2/src/state.rs:115:21
    |
115 | pub struct State<T: Send + Sync + 'static>(T);
    |                     ^^^^ required by this bound in `rocket::State`
inzanez commented 2 years ago

@undeflife Ok, my mistake. It seems that an implementation of Send is sufficient,...

unsafe impl Send for Office {}

Not sure if that's wanted in this case,...

undeflife commented 2 years ago

There is raw pointer in struct Office https://doc.rust-lang.org/std/marker/trait.Send.html#impl-Send-24

I don't think unsafe impl Send for Office {} is a good idea, but in your case, you can create a wrapper struct

struct OfficePtr(Office);

unsafe impl Send for OfficePtr {}
inzanez commented 2 years ago

Ah, that's a good idea, thanks!