Usage of the Send trait in cursive

[fin-ger]

I would like to discuss the usage of the Send trait in the CbSink and View implementations.

Problem description

I am currently working on a reimplementation of the cursive-async-view crate on the feature/no-more-threads-for-you branch.

The implementation on the master branch currently requires the async loaded view to be Send. This is not the case for many cursive views like the SelectView, EditView, Button, LinearLayout, ListView, etc. In order for the AsyncView to be useful, we have to remove the Send restriction for the async loaded view.

A solution would be to use the cursive event loop to regularly check for the async loaded view to be available. However, the cursive event loop requires the callback sent into the CbSink to be Send. This restriction is only needed when the user of a cursive instance wants to use the CbSink in another thread. Cursive itself does not need the callback to be Send as the event loop implementation is completely single-threaded.

In order to allow async creation of views (and usage of views in a CbSink callback) we need to do one of the following:

1. Remove the Send restriction from the CbSink. This will break all code which uses the CbSink in another thread (like many of the provided examples).
2. Add a second CbSink which does not require the callback to be Send. Meaning, there would be CbSendSink = Sender<Box<dyn FnOnce(&mut Cursive) + Send>> and a CbSink = pub type CbSink = Sender<Box<dyn FnOnce(&mut Cursive)>>, where CbSendSink would be usable like the current CbSink and the new CbSink would be usable from within the cursive thread (e.g. via another callback).
3. Make the View trait require a view to be Send. This might have performance impacts as types like Rc and RefCell cannot be used any more. Also, many existing views must be reimplemented to match the new Send requirement, which may introduce new bugs and backward compatibility problems.

Do you have any other ideas or suggestions on how to resolve this problem?

/cc @jwuensche

[gyscos]

I've been trying to avoid requiring Send for views. I don't know how long this can last, but so far there hasn't been a good enough reason to let that go. Usually I have been passing around view-creating closures (which are easier to be Send) rather than the views themselves.

I'm tempted by option 2) where we have a separate queue for non-Send senders; though it seems sharing the receiver isn't easy, and we may need a non-Send receiver (and duplicate the event-processing code)...

Also possibly interesting: https://crates.io/crates/send_wrapper

[fin-ger]

I've been trying to avoid requiring Send for views

I agree, Send should be avoided for views.

I will look into the send_wrapper crate on Friday and see if it can solve my problem. Looks promising though. I would try using it in cursive-async-view directly and report my findings here. Maybe we can integrate a general solution for this problem into cursive itself 😊

Thanks for you're quick reply and awesome project you are running here! 😎

[fin-ger]

I got a POC running with the send_wrapper crate. However, a !Send closure sink would not be useful to me, as I need to enqueue callbacks regularly into the event loop. Currently, this is done in an extra thread which sleeps for 16ms and then enqueues a callback into the event loop. Therefore, my callbacks have a Send restriction, regardless of the cursive sink implementation.

https://github.com/deinstapel/cursive-async-view/blob/feature/no-more-threads-for-you/src/infinite.rs#L197

Is there a better way to enqueue callbacks delayed into cursive?

[gyscos]

I haven't thought about delayed callbacks much yet, but it looks like a good solution so far.
If it becomes a common use-case I may add some solution in cursive for delayed/recurring callbacks... 🤔

Though it now means that the callback is executed in the same thread, and is responsible for doing async request?

I guess one option may be to split async process and view creation:

fn run_async<Callback, R, ViewMaker, V>(callback: Callback, view_maker: ViewMaker)
    where Callback: Send + FnOnce() -> R,
          R: Send,
          ViewMaker: FnOnce(R) -> V,
          V: View,
{ /* ... */ }

run_async(|| slow_download().parse(), MyView::new);

[fin-ger]

I decided against moving a callback into a dedicated thread as some operations like looking for a timeout, waiting for data on a channel, reading a file, etc. can be done "non-blocking" by using methods like try_recv or chunked read from a file. Adding an extra thread for this would consume more resources than needed (e.g. an extra thread which is waiting).

The user can decide whether a dedicated thread is needed. The simple example now shows how to use the AsyncView with a dedicated thread:

https://github.com/deinstapel/cursive-async-view/blob/feature/no-more-threads-for-you/examples/simple.rs

I will create another example which shows how to use the AsyncView without a dedicated thread (and possibly rename the "threaded" example).

Concerning the 3 possibilities mentioned above, I would suggest leaving the API as is. A comment in the documentation of the CbSink could refer to the send_wrapper crate and this issue. If someone encounters a use-case where the send_wrapper crate is not helpful, this issue can be reopened and the possibilities reconsidered.

/cc @jwuensche

[gyscos]

Added a comment in the doc.
I'll close this ticket; we may start another discussion to talk about recurring callbacks and how it may simplify your use-case.