Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command delayed when sent from another thread #646

Open
justinlovinger opened this issue Aug 9, 2023 · 13 comments
Open

Command delayed when sent from another thread #646

justinlovinger opened this issue Aug 9, 2023 · 13 comments

Comments

@justinlovinger
Copy link

Using run_command from the river-control protocol, https://github.com/riverwm/river/blob/c16628c7f57c51d50f2d10a96c265fb0afaddb02/protocol/river-control-unstable-v1.xml, worked as expected when my application was single-threaded. The command was dispatched and my application received the resulting callback and River layout_demand immediately. However, when I moved the run_command code to a separate thread, I noticed odd behavior. The callback and layout_demand would only be received after another event was received in the same queue, as if the other event flushed the queue. As far as I can tell, it was the command being dispatched that was delayed, not the callback being received, because the layout_demand from River that should follow the command was also delayed.

@elinorbgr
Copy link
Member

Could you give some details about what you are doing? In particular what is being done on which thread?

@justinlovinger
Copy link
Author

The main thread runs the main dispatch loop,

fn main() {
    let mut layout_manager = LayoutManager::default();

    let conn = Connection::connect_to_env().unwrap();
    let mut event_queue = conn.new_event_queue();
    // `get_registry` has necessary side-effects.
    let _registry = conn.display().get_registry(&event_queue.handle(), ());
    event_queue.roundtrip(&mut layout_manager).unwrap();
    loop {
        event_queue.blocking_dispatch(&mut layout_manager).unwrap();
    }
}

and everything not handled by the other thread. The other thread is sometimes spawned during a LayoutDemand event. It runs a slow function, caches the result, and sends a command,

let control = Arc::clone(
    state
        .control
        .as_ref()
        .expect("River control should be initialized"),
);
let seat = Arc::clone(
    state.seat.as_ref().expect("seat should be initialized"),
);
let qhandle = qhandle.clone();
thread::spawn(move || {
    let layout = layout(usable_width, usable_height, view_count);
    CACHE.lock().unwrap().insert(key, layout);

    // River will send a new layout demand
    // if it receives a layout command.
    let control = control.lock().unwrap();
    control.add_argument("send-layout-cmd".to_owned());
    control.add_argument("owm".to_owned());
    control.add_argument("retry-layout".to_owned());
    control.run_command(&seat, &qhandle, ());
});

@kchibisov
Copy link
Member

I guess the issue is that you need to flush the connection?

@kchibisov
Copy link
Member

Could you put qhandle.flush() in the end of your control series?

@justinlovinger
Copy link
Author

Could you put qhandle.flush() in the end of your control series?

QueueHandle lacks a flush method. I only see freeze and make_data, https://docs.rs/wayland-client/latest/wayland_client/struct.QueueHandle.html.

@kchibisov
Copy link
Member

Ah, yeah, it's on the queue itself. You could clone Connection, send it to other thread, and flush connection.

@justinlovinger
Copy link
Author

That worked. Although, the question remains, why do I need to flush the connection when sending from another thread?

@kchibisov
Copy link
Member

Because when you do dispatch it does the flush automatically when you go back to it, however when you do that from another thread, you could be waiting on the epoll, but your other thread could send requests, thus they won't be flushed, because the main one is in epoll waiting.

@justinlovinger
Copy link
Author

I see. Perhaps this should be documented more prominently? Maybe a section on multithreading in the wayland-client top-level docs?

@kchibisov
Copy link
Member

Well, the flush docs do say exactly that, but implicitly, they say that you must flush for your requests to reach the server.

@justinlovinger
Copy link
Author

However, a user is unlikely to look for the flush documentation unless they already know they need to flush the connection, and nothing in the top-level doc mentions flushing the connection.

@kchibisov
Copy link
Member

I mean, I don't deny that more obvious docs could be better, it's up to @elinorbgr in the end.

@elinorbgr
Copy link
Member

Noted, we can improve the toplevel docs to make the need for flushing more obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants