gstreamr: bus: Add BusWatchGuard to automatically remove watch

Previously, with add_watch()/add_watch_local() you had to remember
calling remove_watch() in order not to leak the bus, the watch source
and two associated file descriptors. Now these methods instead return an
object of type BusWatchGuard that will automatically remove the bus
watch when the object is dropped.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/1248>

examples/src/bin/custom_events.rs

@@ -133,7 +133,8 @@ fn example_main() {
     // Every message from the bus is passed through this function. Its returnvalue determines
     // whether the handler wants to be called again. If glib::Continue(false) is returned, the
     // handler is removed and will never be called again. The mainloop still runs though.
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let main_loop = &main_loop_clone;
@@ -169,11 +170,6 @@ fn example_main() {
     pipeline
         .set_state(gst::State::Null)
         .expect("Unable to set the pipeline to the `Null` state");
-
-    // Remove the watch function from the bus.
-    // Again: There can always only be one watch function.
-    // Thus we don't have to tell him which function to remove.
-    bus.remove_watch().unwrap();
 }
 
 fn main() {

examples/src/bin/d3d11videosink.rs

@@ -329,7 +329,8 @@ fn main() -> Result<()> {
 
     let main_loop_clone = main_loop.clone();
     let bus = playbin.bus().unwrap();
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let main_loop = &main_loop_clone;

examples/src/bin/events.rs

@@ -87,7 +87,8 @@ fn example_main() {
     // Every message from the bus is passed through this function. Its returnvalue determines
     // whether the handler wants to be called again. If glib::Continue(false) is returned, the
     // handler is removed and will never be called again. The mainloop still runs though.
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let main_loop = &main_loop_clone;
@@ -123,11 +124,6 @@ fn example_main() {
     pipeline
         .set_state(gst::State::Null)
         .expect("Unable to set the pipeline to the `Null` state");
-
-    // Remove the watch function from the bus.
-    // Again: There can always only be one watch function.
-    // Thus we don't have to tell him which function to remove.
-    bus.remove_watch().unwrap();
 }
 
 fn main() {

examples/src/bin/gtksink.rs

@@ -103,7 +103,8 @@ fn create_ui(app: &gtk::Application) {
         .expect("Unable to set the pipeline to the `Playing` state");
 
     let app_weak = app.downgrade();
-    bus.add_watch_local(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch_local(move |_, msg| {
             use gst::MessageView;
 
             let app = match app_weak.upgrade() {
@@ -149,7 +150,6 @@ fn create_ui(app: &gtk::Application) {
             pipeline
                 .set_state(gst::State::Null)
                 .expect("Unable to set the pipeline to the `Null` state");
-            pipeline.bus().unwrap().remove_watch().unwrap();
         }
 
         if let Some(timeout_id) = timeout_id.borrow_mut().take() {

examples/src/bin/gtkvideooverlay.rs

@@ -204,7 +204,8 @@ fn create_ui(app: &gtk::Application) {
         .expect("Unable to set the pipeline to the `Playing` state");
 
     let app_weak = app.downgrade();
-    bus.add_watch_local(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch_local(move |_, msg| {
             use gst::MessageView;
 
             let app = match app_weak.upgrade() {
@@ -250,7 +251,6 @@ fn create_ui(app: &gtk::Application) {
             pipeline
                 .set_state(gst::State::Null)
                 .expect("Unable to set the pipeline to the `Null` state");
-            pipeline.bus().unwrap().remove_watch().unwrap();
         }
 
         if let Some(timeout_id) = timeout_id.borrow_mut().take() {

examples/src/bin/launch_glib_main.rs

@@ -35,7 +35,8 @@ fn example_main() {
 
     //bus.add_signal_watch();
     //bus.connect_message(None, move |_, msg| {
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let main_loop = &main_loop_clone;
@@ -62,11 +63,6 @@ fn example_main() {
     pipeline
         .set_state(gst::State::Null)
         .expect("Unable to set the pipeline to the `Null` state");
-
-    // Here we remove the bus watch we added above. This avoids a memory leak, that might
-    // otherwise happen because we moved a strong reference (clone of main_loop) into the
-    // callback closure above.
-    bus.remove_watch().unwrap();
 }
 
 fn main() {

examples/src/bin/queries.rs

@@ -94,7 +94,8 @@ fn example_main() {
     let main_loop_clone = main_loop.clone();
     //bus.add_signal_watch();
     //bus.connect_message(None, move |_, msg| {
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let main_loop = &main_loop_clone;
@@ -122,7 +123,6 @@ fn example_main() {
         .set_state(gst::State::Null)
         .expect("Unable to set the pipeline to the `Null` state");
 
-    bus.remove_watch().unwrap();
     timeout_id.remove();
 }
 

gstreamer/Gir.toml

@@ -586,6 +586,7 @@ final_type = true
 
     [[object.function]]
     name = "remove_watch"
+    visibility = "crate"
         [object.function.return]
         bool_return_is_error = "Bus has no event source"
 

gstreamer/src/auto/bus.rs

@@ -92,7 +92,8 @@ impl Bus {
     }
 
     #[doc(alias = "gst_bus_remove_watch")]
-    pub fn remove_watch(&self) -> Result<(), glib::error::BoolError> {
+    #[allow(dead_code)]
+    pub(crate) fn remove_watch(&self) -> Result<(), glib::error::BoolError> {
         unsafe {
             glib::result_from_gboolean!(
                 ffi::gst_bus_remove_watch(self.to_glib_none().0),

gstreamer/src/bus.rs

@@ -13,7 +13,7 @@ use futures_util::{stream::FusedStream, StreamExt};
 use glib::{
     ffi::{gboolean, gpointer},
     prelude::*,
-    source::{Continue, Priority, SourceId},
+    source::{Continue, Priority},
     translate::*,
 };
 
@@ -135,7 +135,7 @@ impl Bus {
 
     #[doc(alias = "gst_bus_add_watch")]
     #[doc(alias = "gst_bus_add_watch_full")]
-    pub fn add_watch<F>(&self, func: F) -> Result<SourceId, glib::BoolError>
+    pub fn add_watch<F>(&self, func: F) -> Result<BusWatchGuard, glib::BoolError>
     where
         F: FnMut(&Bus, &Message) -> Continue + Send + 'static,
     {
@@ -151,14 +151,14 @@ impl Bus {
             if res == 0 {
                 Err(glib::bool_error!("Bus already has a watch"))
             } else {
-                Ok(from_glib(res))
+                Ok(BusWatchGuard { bus: self.clone() })
             }
         }
     }
 
     #[doc(alias = "gst_bus_add_watch")]
     #[doc(alias = "gst_bus_add_watch_full")]
-    pub fn add_watch_local<F>(&self, func: F) -> Result<SourceId, glib::BoolError>
+    pub fn add_watch_local<F>(&self, func: F) -> Result<BusWatchGuard, glib::BoolError>
     where
         F: FnMut(&Bus, &Message) -> Continue + 'static,
     {
@@ -179,7 +179,7 @@ impl Bus {
             if res == 0 {
                 Err(glib::bool_error!("Bus already has a watch"))
             } else {
-                Ok(from_glib(res))
+                Ok(BusWatchGuard { bus: self.clone() })
             }
         }
     }
@@ -378,6 +378,22 @@ impl FusedStream for BusStream {
     }
 }
 
+// rustdoc-stripper-ignore-next
+/// Manages ownership of the bus watch added to a bus with [`Bus::add_watch`] or [`Bus::add_watch_local`]
+///
+/// When dropped the bus watch is removed from the bus.
+#[derive(Debug)]
+#[must_use = "if unused the bus watch will immediately be removed"]
+pub struct BusWatchGuard {
+    bus: Bus,
+}
+
+impl Drop for BusWatchGuard {
+    fn drop(&mut self) {
+        let _ = self.bus.remove_watch();
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::sync::{Arc, Mutex};

tutorials/src/bin/basic-tutorial-12.rs

@@ -23,7 +23,8 @@ fn tutorial_main() -> Result<(), Error> {
     let main_loop_clone = main_loop.clone();
     let pipeline_weak = pipeline.downgrade();
     let bus = pipeline.bus().expect("Pipeline has no bus");
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             let pipeline = match pipeline_weak.upgrade() {
                 Some(pipeline) => pipeline,
                 None => return glib::Continue(true),
@@ -78,7 +79,6 @@ fn tutorial_main() -> Result<(), Error> {
 
     main_loop.run();
 
-    bus.remove_watch()?;
     pipeline.set_state(gst::State::Null)?;
 
     Ok(())

tutorials/src/bin/playback-tutorial-1.rs

@@ -135,7 +135,7 @@ fn tutorial_main() -> Result<(), Error> {
     let playbin_clone = playbin.clone();
     let main_loop_clone = main_loop.clone();
     let bus = playbin.bus().unwrap();
-    bus.add_watch(move |_bus, message| {
+    let _bus_watch = bus.add_watch(move |_bus, message| {
         use gst::MessageView;
         match message.view() {
             MessageView::Error(err) => {

tutorials/src/bin/playback-tutorial-2.rs

@@ -140,7 +140,7 @@ fn tutorial_main() -> Result<(), Error> {
     let playbin_clone = playbin.clone();
     let main_loop_clone = main_loop.clone();
     let bus = playbin.bus().unwrap();
-    bus.add_watch(move |_bus, message| {
+    let _bus_watch = bus.add_watch(move |_bus, message| {
         use gst::MessageView;
         match message.view() {
             MessageView::Error(err) => {

tutorials/src/bin/playback-tutorial-4.rs

@@ -52,7 +52,8 @@ fn tutorial_main() -> Result<(), Error> {
     let main_loop_clone = main_loop.clone();
     let pipeline_weak = pipeline.downgrade();
     let bus = pipeline.bus().unwrap();
-    bus.add_watch(move |_, msg| {
+    let _bus_watch = bus
+        .add_watch(move |_, msg| {
             use gst::MessageView;
 
             let buffering_level = &buffering_level_clone;
@@ -177,7 +178,6 @@ fn tutorial_main() -> Result<(), Error> {
     // Shutdown pipeline
     pipeline.set_state(gst::State::Null)?;
 
-    bus.remove_watch()?;
     timeout_id.remove();
 
     Ok(())

tutorials/src/bin/playback-tutorial-5.rs

@@ -145,7 +145,7 @@ fn tutorial_main() -> Result<(), Error> {
     let main_loop_clone = main_loop.clone();
     let bus = pipeline.bus().unwrap();
     let pipeline_weak = pipeline.downgrade();
-    bus.add_watch(move |_bus, message| {
+    let _bus_watch = bus.add_watch(move |_bus, message| {
         use gst::MessageView;
 
         let pipeline = match pipeline_weak.upgrade() {