validate: Give a mutable reference to the action in the execute function

We need to be able to keep a reference to the action when we
implement async action types so now we can do `action.clone()`
and get a hard reference to it.

We also need to be able to mutate the structure, it is possible to
`get_mut()` on it, get a mutable reference to the structure.

There was a bug in the test where we were using a ref to the wrong
action object in the async signal which is why we didn't detect the
problem.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/1586>
This commit is contained in:
Thibault Saunier 2024-10-30 18:20:19 -03:00 committed by GStreamer Marge Bot
parent 5fccc0bc82
commit d8a3784b74
3 changed files with 50 additions and 21 deletions

View file

@ -47,6 +47,22 @@ impl ActionRef {
}
impl Action {
pub(crate) unsafe fn from_glib_ptr_borrow_mut(
ptr: &mut *mut ffi::GstValidateAction,
) -> &mut Self {
assert_initialized_main_thread!();
debug_assert_eq!((*(*ptr)).mini_object.refcount, 1);
debug_assert_eq!(
std::mem::size_of::<Action>(),
std::mem::size_of::<gst::glib::ffi::gpointer>()
);
debug_assert!(!ptr.is_null());
&mut *(ptr as *mut *mut ffi::GstValidateAction as *mut Action)
}
#[doc(alias = "gst_validate_action_new")]
pub fn new(
scenario: Option<&impl IsA<Scenario>>,

View file

@ -1,7 +1,7 @@
use std::{ffi::c_int, ptr};
use crate::ffi;
use glib::{object::Cast, translate::*};
use crate::{ffi, Action};
use glib::translate::*;
#[derive(Debug)]
#[repr(transparent)]
@ -176,7 +176,7 @@ impl<'a> ActionParameterBuilder<'a> {
}
}
type ActionFunction = dyn Fn(&crate::Scenario, &mut crate::ActionRef) -> Result<crate::ActionSuccess, crate::ActionError>
type ActionFunction = dyn Fn(&crate::Scenario, &mut crate::Action) -> Result<crate::ActionSuccess, crate::ActionError>
+ Sync
+ Send
+ 'static;
@ -198,7 +198,7 @@ impl<'a> ActionTypeBuilder<'a> {
pub fn new<
F: Fn(
&crate::Scenario,
&mut crate::ActionRef,
&mut crate::Action,
) -> Result<crate::ActionSuccess, crate::ActionError>
+ Send
+ Sync
@ -318,9 +318,9 @@ impl<'a> ActionTypeBuilder<'a> {
unsafe extern "C" fn execute_func_trampoline(
scenario: *mut ffi::GstValidateScenario,
action: *mut ffi::GstValidateAction,
mut action_ptr: *mut ffi::GstValidateAction,
) -> c_int {
let action_type = ffi::gst_validate_get_action_type((*action).type_);
let action_type = ffi::gst_validate_get_action_type((*action_ptr).type_);
let scenario = from_glib_borrow(scenario);
let func: &ActionFunction = &*(gst::ffi::gst_mini_object_get_qdata(
@ -328,21 +328,21 @@ impl<'a> ActionTypeBuilder<'a> {
QUARK_ACTION_TYPE_FUNC.get().unwrap().into_glib(),
) as *const Box<ActionFunction>);
let res = (*func)(&scenario, crate::ActionRef::from_mut_ptr(action));
// SAFETY: `execute_func_trampoline` is called with the unic reference of `action_ptr`
// so we can safely borrow it mutably
let original_ptr = action_ptr;
let action = Action::from_glib_ptr_borrow_mut(&mut action_ptr);
let res = (*func)(&scenario, action);
if let Err(crate::ActionError::Error(ref err)) = res {
scenario
.dynamic_cast_ref::<crate::Reporter>()
.unwrap()
.report_action(
&from_glib_borrow(action),
glib::Quark::from_str("scenario::execution-error"),
err,
);
return ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED;
debug_assert_eq!(action.as_ptr(), original_ptr);
match res {
Err(crate::ActionError::Error(ref err)) => {
action.report_error(err);
ffi::GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED
}
Err(_) => panic!("New action error types should be handled here."),
Ok(v) => v.into_glib(),
}
res.into_glib()
}
unsafe {

View file

@ -89,7 +89,6 @@ fn test_action_types() {
assert!(!*fails_called.lock().unwrap());
action.execute().expect_err("Action should have failed");
assert!(*fails_called.lock().unwrap());
action.set_done();
gst_validate::ActionParameterBuilder::new("async", "Verify unused param are properly cleaned")
.default_value("true")
@ -102,12 +101,26 @@ fn test_action_types() {
glib::clone!(
#[strong]
async_called,
move |_, _action| {
move |_, action| {
let action_mut = action.get_mut().unwrap();
action_mut
.structure_mut()
.expect("We should have a structure set in that action")
.set("running-async", true);
std::thread::spawn(glib::clone!(
#[strong]
async_called,
#[strong]
action,
move || {
*async_called.0.lock().unwrap() = true;
assert!(action
.structure()
.unwrap()
.get::<bool>("running-async")
.unwrap());
action.set_done();
}
));