rust-jack icon indicating copy to clipboard operation
rust-jack copied to clipboard

remove Sync requirement from NotificationHandler

Open BillyDM opened this issue 3 years ago • 7 comments
trafficstars

The Sync requirement on NotificationHandler is unecessary, and it is preventing me from storing an mpsc::Sender in my struct.

BillyDM avatar Feb 19 '22 16:02 BillyDM

Also, I made self in NotificationHandler::thread_init mutable so I can actually use my mpsc::Sender to send a message to my GUI.

BillyDM avatar Feb 19 '22 17:02 BillyDM

I'll have to double check that this is valid. If this is not valid, then you will probably have to use a Mutex and/or SyncSender.

wmedrano avatar Feb 21 '22 02:02 wmedrano

I have also stumbled over this.

Be-ing avatar Feb 22 '22 17:02 Be-ing

Also, I made self in NotificationHandler::thread_init mutable so I can actually use my mpsc::Sender to send a message to my GUI.

Due to #104 I made a small test function:

    fn thread_init(&self, _: &Client) {
        info!("thread_init enter {:?}", thread::current().id());
        thread::sleep(Duration::from_secs(1));
        info!("thread_init exit {:?}", thread::current().id());
    }

..which gives the following output:

2023-08-13 21:12:56.358413 INFO [jack_connector::my_jack] thread_init enter ThreadId(6)
2023-08-13 21:12:57.358612 INFO [jack_connector::my_jack] thread_init exit ThreadId(6)
2023-08-13 21:12:57.358849 INFO [jack_connector::my_jack] thread_init enter ThreadId(7)
2023-08-13 21:12:57.368143 INFO [jack_connector::my_jack] thread_init enter ThreadId(8)
2023-08-13 21:12:58.359022 INFO [jack_connector::my_jack] thread_init exit ThreadId(7)
2023-08-13 21:12:58.368318 INFO [jack_connector::my_jack] thread_init exit ThreadId(8)

.. which reveals two threads (7 and 8) to be running thread_init() simultaneously. Therefore giving mutable access to self will yield undefined behaviour.

The Sync requirement on NotificationHandler is unecessary, and it is preventing me from storing an mpsc::Sender in my struct.

I think the #121 findings should be investigated thoroughly before concluding that only a single method of the NotificationHandler trait will ever be running at any given time, despite being called in different threads.

The findings also makes me wonder whether it is safe to have &mut self passed to the other NotificationHandler callbacks than thread_init..

And in general I think the point that jack has 2 (or is pipewire a 3rd?) implementations also needs to be considered.

xkr47 avatar Aug 13 '23 18:08 xkr47