Re: [dm-devel] [PATCH v2 28/37] multipath-tools tests: mock pthread_mutex_{lock, unlock}

[Benjamin Marzinski <bmarzins@redhat.com>]

On Mon, Sep 11, 2023 at 06:38:37PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> If some test fails with a lock held, cmocka doesn't deal well with
> pthread_cleanup_pop(). Such tests can cause deadlock with the locking
> primitives in the alias code, because locks don't get properly unlocked.  Just
> mock the lock/unlock functions and generate an error if they weren't paired at
> the end of the test.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  tests/Makefile |  1 +
>  tests/alias.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index c777d07..7dac8a8 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -52,6 +52,7 @@ blacklist-test_LIBDEPS := -ludev
>  vpd-test_OBJDEPS :=  $(multipathdir)/discovery.o
>  vpd-test_LIBDEPS := -ludev -lpthread -ldl
>  alias-test_TESTDEPS := test-log.o
> +alias-test_OBJDEPS := $(mpathutildir)/util.o
>  alias-test_LIBDEPS := -lpthread -ldl
>  valid-test_OBJDEPS := $(multipathdir)/valid.o $(multipathdir)/discovery.o
>  valid-test_LIBDEPS := -lmount -ludev -lpthread -ldl
> diff --git a/tests/alias.c b/tests/alias.c
> index 872b1fc..962c158 100644
> --- a/tests/alias.c
> +++ b/tests/alias.c
> @@ -89,6 +89,47 @@ int __wrap_dm_get_uuid(const char *name, char *uuid, int uuid_len)
>  	return ret;
>  }
>  
> +static int lock_errors;
> +static int bindings_locked;
> +static int timestamp_locked;
> +int __wrap_pthread_mutex_lock(pthread_mutex_t *mutex)
> +{
> +	if (mutex == &bindings_mutex) {
> +		if (bindings_locked) {
> +			fprintf(stderr, "%s: bindings_mutex LOCKED\n", __func__);
> +			lock_errors++;
> +		}
> +		bindings_locked = 1;
> +	}  else if (mutex == &timestamp_mutex) {
> +		if (timestamp_locked) {
> +			fprintf(stderr, "%s: timestamp_mutex LOCKED\n", __func__);
> +			lock_errors++;
> +		}
> +		timestamp_locked = 1;
> +	} else
> +		  fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex);
> +	return 0;
> +}
> +
> +int __wrap_pthread_mutex_unlock(pthread_mutex_t *mutex)
> +{
> +	if (mutex == &bindings_mutex) {
> +		if (!bindings_locked) {
> +			fprintf(stderr, "%s: bindings_mutex UNLOCKED\n", __func__);
> +			lock_errors++;
> +		}
> +		bindings_locked = 0;
> +	}  else if (mutex == &timestamp_mutex) {
> +		if (!timestamp_locked) {
> +			fprintf(stderr, "%s: timestamp_mutex UNLOCKED\n", __func__);
> +			lock_errors++;
> +		}
> +		timestamp_locked = 0;
> +	} else
> +		  fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex);
> +	return 0;
> +}
> +
>  #define TEST_FDNO 1234
>  #define TEST_FPTR ((FILE *) 0xaffe)
>  
> @@ -1718,6 +1759,10 @@ static void gufa_old_nomatch_nowwidmatch(void **state) {
>  	free(alias);
>  }
>  
> +static void gufa_check_locking(void **state) {
> +	assert_int_equal(lock_errors, 0);
> +}
> +
>  static int test_get_user_friendly_alias()
>  {
>  	const struct CMUnitTest tests[] = {
> @@ -1743,6 +1788,7 @@ static int test_get_user_friendly_alias()
>  		cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch, teardown_bindings),
>  		cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch_used, teardown_bindings),
>  		cmocka_unit_test_teardown(gufa_old_nomatch_nowwidmatch, teardown_bindings),
> +		cmocka_unit_test(gufa_check_locking),
>  	};
>  
>  	return cmocka_run_group_tests(tests, NULL, NULL);
> -- 
> 2.42.0
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel