Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ammar Faizi <ammarfaizi2@gnuweeb.org>
To:     Jens Axboe <axboe@kernel.dk>
Cc:     Ammar Faizi <ammarfaizi2@gnuweeb.org>,
        =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <pobrn@protonmail.com>,
        Michael William Jonathan <moe@gnuweeb.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        io-uring Mailing List <io-uring@vger.kernel.org>,
        GNU/Weeb Mailing List <gwml@vger.gnuweeb.org>
Subject: [PATCH liburing v1 2/2] recv-msgall: Fix invalid mutex usage
Date:   Wed, 10 May 2023 21:39:27 +0700
Message-Id: <20230510143927.123170-3-ammarfaizi2@gnuweeb.org>
In-Reply-To: <20230510143927.123170-1-ammarfaizi2@gnuweeb.org>
References: <20230510143927.123170-1-ammarfaizi2@gnuweeb.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Calling pthread_mutex_lock() twice with the same mutex in the same
thread without unlocking it first is invalid. The intention behind this
pattern was to wait for the recv_fn() thread to be ready.

Use the pthread barrier instead. It is more straightforward and correct.

Fixes: https://github.com/axboe/liburing/issues/855
Reported-by: Barnabás Pőcze <pobrn@protonmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/recv-msgall.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/test/recv-msgall.c b/test/recv-msgall.c
index f809834b2e427fc5..d1fcdb0d510423e7 100644
--- a/test/recv-msgall.c
+++ b/test/recv-msgall.c
@@ -19,7 +19,7 @@
 #define HOST	"127.0.0.1"
 static __be16 bind_port;
 struct recv_data {
-	pthread_mutex_t mutex;
+	pthread_barrier_t barrier;
 	int use_recvmsg;
 	struct msghdr msg;
 };
@@ -122,11 +122,11 @@ static void *recv_fn(void *data)
 
 	ret = t_create_ring_params(1, &ring, &p);
 	if (ret == T_SETUP_SKIP) {
-		pthread_mutex_unlock(&rd->mutex);
+		pthread_barrier_wait(&rd->barrier);
 		ret = 0;
 		goto err;
 	} else if (ret < 0) {
-		pthread_mutex_unlock(&rd->mutex);
+		pthread_barrier_wait(&rd->barrier);
 		goto err;
 	}
 
@@ -135,7 +135,7 @@ static void *recv_fn(void *data)
 		fprintf(stderr, "recv_prep failed: %d\n", ret);
 		goto err;
 	}
-	pthread_mutex_unlock(&rd->mutex);
+	pthread_barrier_wait(&rd->barrier);
 	ret = do_recv(&ring);
 	close(sock);
 	io_uring_queue_exit(&ring);
@@ -219,28 +219,24 @@ err:
 
 static int test(int use_recvmsg)
 {
-	pthread_mutexattr_t attr;
 	pthread_t recv_thread;
 	struct recv_data rd;
 	int ret;
 	void *retval;
 
-	pthread_mutexattr_init(&attr);
-	pthread_mutexattr_setpshared(&attr, 1);
-	pthread_mutex_init(&rd.mutex, &attr);
-	pthread_mutex_lock(&rd.mutex);
+	pthread_barrier_init(&rd.barrier, NULL, 2);
 	rd.use_recvmsg = use_recvmsg;
 
 	ret = pthread_create(&recv_thread, NULL, recv_fn, &rd);
 	if (ret) {
 		fprintf(stderr, "Thread create failed: %d\n", ret);
-		pthread_mutex_unlock(&rd.mutex);
 		return 1;
 	}
 
-	pthread_mutex_lock(&rd.mutex);
+	pthread_barrier_wait(&rd.barrier);
 	do_send();
 	pthread_join(recv_thread, &retval);
+	pthread_barrier_destroy(&rd.barrier);
 	return (intptr_t)retval;
 }
 
-- 
Ammar Faizi