pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-uecups/+/40798?usp=email )
Change subject: cosmetic: Improve comments on cancellable thread safety
......................................................................
cosmetic: Improve comments on cancellable thread safety
* Move the comment explaining safety measures to the top of the thread
function
* Document that special care must be taken in mutual exclusion zones
within the thread code.
Change-Id: Ic1ffe68c1637cb06787d4193347cb4200c819154
---
M daemon/gtp_endpoint.c
M daemon/tun_device.c
2 files changed, 21 insertions(+), 10 deletions(-)
Approvals:
osmith: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/daemon/gtp_endpoint.c b/daemon/gtp_endpoint.c
index 7892d4e..87d333c 100644
--- a/daemon/gtp_endpoint.c
+++ b/daemon/gtp_endpoint.c
@@ -124,11 +124,16 @@
}
}
-/* one thread for reading from each GTP/UDP socket (GTP decapsulation -> tun)
- * IMPORTANT!: All logging functions in this function block must be called with
- * PTHREAD_CANCEL_DISABLE set, otherwise the thread could be cancelled while
- * holding the logging mutex, hence causing deadlock with main (or other)
- * thread. */
+/* One thread for reading from each GTP/UDP socket (GTP decapsulation -> tun)
+ * IMPORTANT!: Since this thread is cancellable (deferred type):
+ * - All osmo logging functions in this thread must be called with
PTHREAD_CANCEL_DISABLE set,
+ * otherwise the thread could be cancelled while holding the libosmocore
logging mutex, hence causing
+ * deadlock with main (or other) thread.
+ * - Within pthread_rwlock_*(&d->rwlock) mutual exclusion zone, if we ever do
any call considered
+ * a cancellation point (see "man pthreads"), then make sure to do the call
protected with
+ * PTHREAD_CANCEL_DISABLE set, otherwise we may leave the d->rwlock held
forever and cause a deadlock
+ * with main (or other) thread.
+ */
static void *gtp_endpoint_thread(void *arg)
{
struct gtp_endpoint *ep = (struct gtp_endpoint *)arg;
diff --git a/daemon/tun_device.c b/daemon/tun_device.c
index ee4fbcc..40b6ec3 100644
--- a/daemon/tun_device.c
+++ b/daemon/tun_device.c
@@ -212,7 +212,16 @@
return rc;
}
-/* one thread for reading from each TUN device (TUN -> GTP encapsulation) */
+/* One thread for reading from each TUN device (TUN -> GTP encapsulation)
+ * IMPORTANT!: Since this thread is cancellable (deferred type):
+ * - All osmo logging functions in this thread must be called with
PTHREAD_CANCEL_DISABLE set,
+ * otherwise the thread could be cancelled while holding the libosmocore
logging mutex, hence causing
+ * deadlock with main (or other) thread.
+ * - Within pthread_rwlock_*(&d->rwlock) mutual exclusion zone, if we ever do
any call considered
+ * a cancellation point (see "man pthreads"), then make sure to do the call
protected with
+ * PTHREAD_CANCEL_DISABLE set, otherwise we may leave the d->rwlock held
forever and cause a deadlock
+ * with main (or other) thread.
+ */
static void *tun_device_thread(void *arg)
{
struct tun_device *tun = (struct tun_device *)arg;
@@ -223,10 +232,6 @@
uint8_t base_buffer[payload_off_4byte_aligned + MAX_UDP_PACKET];
pthread_cleanup_push(tun_device_pthread_cleanup_routine, tun);
- /* IMPORTANT!: All logging functions in this function block must be
called with
- * PTHREAD_CANCEL_DISABLE set, otherwise the thread could be cancelled
while
- * holding the logging mutex, hence causing deadlock with main (or
other)
- * thread. */
snprintf(thread_name, sizeof(thread_name), "Rx%s", tun->devname);
pthread_setname_np(pthread_self(), thread_name);
@@ -269,6 +274,7 @@
continue;
}
rc = tx_gtp1u_pkt(t, base_buffer, buffer, nread);
+ /* pthread_rwlock_unlock() was called inside tx_gtp1u_pkt(). */
if (rc < 0) {
LOGTUN_NC(tun, LOGL_FATAL, "Error Writing to UDP
socket: %s\n", strerror(errno));
exit(1);
--
To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/40798?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-uecups
Gerrit-Branch: master
Gerrit-Change-Id: Ic1ffe68c1637cb06787d4193347cb4200c819154
Gerrit-Change-Number: 40798
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
