Overwhelmed by the complexity of using closures in threads

help
1

Despite the fact that i recently increased my overall knowledge about rust and borrow checker considerably by reading dozens of documents and videos, and fixed several of my basic issues with closure and threads with the help of the community i still struggle to get my opensource project to work. it seems that using closures in concurrent threads is way above my current rust skill level. The project is small so i though maybe it's better to ask someone to have a look.

The problem is i can comfortably use closures in concurrent threads but when i try to do the same with a level of abstraction and some crates, rust starts complaining about the closure life time, mismatched types, etc

Here is what my programs does
it monitors the clipboard and if its content is text clears it after few seconds but keeps the value in a sort of history. there is also a hotkey that will restore the last text in history into the clipboard.

  • There is module clipboard_holder with ClipboardHolder struc that is responsible with managing the history. ClipboardHolder has a method that returns a closure that can be used in threads for send the clipboard content using crossbeam channel.
    clipboard holder also spawns a thread internally to receive clipboard texts sent by the crossbeam channel's senders
  • There is a thread that calls a function in the clipboard module to monitors clipboard and calls the clipboard holder closure before clearing the to send its content to clipboard holder
  • In the main thread there is also a hotkey listener that is supposed the call clipboard holder for restoring the last clipboard content (it only prints something right now)

I wand to make the closure that calls the ClipboardHolder as flexible as possible so i can use it in several different concurrent threads and pass it inside other functions as well

i hope someone can have a look and help me out
In the last commit i tried to add more abstraction and use 'a suggested in the forum (Callback calling a type method - #3 by kornel), it did solve my abstraction problem but now i get another error
this is exactly the kind of problem i have. it works in an isolated example but as i as move it to my problem i get encounter errors. previously i use 'static and box but that also did solve all the issues

implicit elided lifetime not allowed here

project page : GitHub - sinasalek/clipboard_guardian: Protects sensitive content in clipboard

2

What kind of abstractions are you introducing? Functions that return closures, or something else? And how are you sharing the objects that you are accessing from multiple threads?

As for you elided lifetime error, can you post the exact function signature that caused it?

3

I moved all the things related to clipboard history management to the clipboard_holder module (https://github.com/sinasalek/clipboard_guardian/blob/master/src/clipboard_holder.rs)
And just create an instance of it and call sender_callback_create to create the callback for passing it to the threads
let clipboard_holder = clipboard_holder::ClipboardHolder::new();
let callback = clipboard_holder.sender_callback_create();

// Setup a separate thread to monitor clipboard changes
thread::spawn(move || {
    clipboard::clipboard_change_monitor(thread_cfg.interval, callback.clone());
});

and the sender_callback_create method code:

    pub fn sender_callback_create(&self) -> CallbackType<'_> {
       Arc::new(move | text: &String | { self.safe_history_add(Box::new(text)); })
    }

Regarding elided lifetime. the type is this

pub type CallbackType<'a> = Arc<dyn Fn(&String) + Send + Sync+ 'a>;

And this is the code that causes the current error:

struct Handler<'a> {
   clipboard_sender: CallbackType<'a>
}
4

Well the self parameter in sender_callback_create is a reference, so if you move the reference into a closure, that closure now borrows from self, which means you can't move the closure to other threads.

2 Likes
5

that is true, safe_history_add only calls crossbeam channel so i can probably safely remove move there as well as mut from safe_history_add. i did try that and made a new commit. now i have this error:

error[E0597]: clipboard_holder does not live long enough
--> src\main.rs:13:20

The clipboard_holder is in the main thread and it never ends. so it should outlive the threads. i tried adding static to the new method in ClipboardHolder but that wasn't the right of doing it. How do i tell rust that clipboard_holder should live till the end of program?

6

I recommend not moving references into the closure at all.

pub type CallbackType = Arc<dyn Fn(String) + Send + Sync>;

impl ClipboardHolder {
    pub fn sender_callback_create(&self) -> CallbackType {
        let history_sender = self.history_sender.clone();
        Arc::new(move |text| { history_sender.send(text.to_string()); })
    }
}

Also, I spotted the type Box<&String> which makes no sense whatsoever. Just use &str. In the above I changed the closure to take a String, since the closure just clones the string slice otherwise.

You might also be able to do without the Arc.

impl ClipboardHolder {
    pub fn sender_callback_create(&self) -> impl Fn(String) {
        let history_sender = self.history_sender.clone();
        move |text| { history_sender.send(text.to_string()); }
    }
}
1 Like
7

It's simple program and i can do as you suggested. i'll give it a try
but what if have to call that method and therefore have to move the self reference into the closure?
How do i tell rust the clipboard_holder is for example static?
I tried this:

let clipboard_holder: &'static clipboard_holder::ClipboardHolder = &clipboard_holder::ClipboardHolder::new();

But got this error

temporary value dropped while borrowed

8

You don't "tell the compiler that it is static". Lifetimes come from the shape of your code, so to change the lifetimes, you must change the shape of your code. The only way to make a static reference at runtime is permanently leak the memory (i.e. by changing the shape of the code so it lives forever).

If you need to access something in the closure, you must use some sort of clone to do so. One common strategy is to make a ClipboardHolderInner with your fields and to make the ClipboardHolder just have a single field of type Arc<ClipboardHolderInner>. Of course, now you need mutexes or atomics to change the fields, but that's the price of multi-threaded code.

2 Likes
9

So using 'static is considered memory leak? and should be avoided?

What i don't understand is why rust complains about clipboard_holder lifetime. i'm borrowing it and it live in the main thread and this code main thread outlives all the other threads. i understand sometimes rust can't be sure about that and so it throws an error.
I implemented what you suggested but i still get the same lifetime error. i can move it to the closure but that's not what i'm looking for. i want to be able to access the clipboradholder in the main thread and use its other methods on the other threads
:

pub struct ClipboardHolderContainer {
	clipboard_holder: Arc<ClipboardHolder>
}

impl ClipboardHolderContainer {
	pub fn new() -> Self {
		ClipboardHolderContainer {
			clipboard_holder: Arc::new(ClipboardHolder::new())
		}
	}

	pub fn sender_callback_create(&self) -> CallbackType<'_> {
		let h = self.clipboard_holder.clone();
		Arc::new(| text: &String | { h.safe_history_add(Box::new(text)); })
	}
}
10

No, static references also applies to immutable compile time constants such as string literals, which is fine. It's just that once the program has started, creating a new static reference is not possible without leaking memory.

Static doesn't have anything to do with it being in the main thread or not. Even variables declared directly in main are not static. A static variable must live until after main returns.

In this case the issue is that you didn't remove the lifetime from CallbackType, so the caller still sees it as if it borrowed from self, even if it doesn't.

2 Likes
11

Thanks i removed all life lifetime signs and i no longer get that error.
I made some small changes as well to finally getting it to compile!! never though this day would come :slight_smile:
Still a long way to go but at least i made a progress after several difficult days
Thank a lot for the help

1 Like
12

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.