pespin has uploaded this change for review.
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(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-uecups refs/changes/98/40798/1
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 change 40798. To unsubscribe, or for help writing mail filters, visit settings.