Re: pthread cancellation points missing for fdatasync and lockf in some cases

[Mike Frysinger <vapier at gentoo dot org>]

On Saturday 05 May 2007, Mike Frysinger wrote:
> i wanted to make sure these things werent done on purpose first ... but
> i'll post a patch

patch attached ... ive updated tst-cancel4 for fdatasync, but i didnt update 
tst-cancel16 for lockf as that test relies on lockf blocking and none of the 
other options passed to lockf will block ...

i'll tag a bug report as well
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

05-06-2007  Mike Frysinger  <vapier@gentoo.org>

	* sysdeps/unix/make-syscalls.sh: Document "C" syscall signature.
	* sysdeps/unix/sysv/linux/syscalls.list (fdatasync): Add "C" to args.
	* io/lockf.c: Handle pthread cancellation.
	* nptl/tst-cancel-wrappers.sh: Set C["fdatasync"] and C["lockf"] to 1.
	* nptl/tst-cancel4.c (tf_fdatasync): New test.

--- sysdeps/unix/make-syscalls.sh
+++ sysdeps/unix/make-syscalls.sh
@@ -10,6 +10,7 @@
 # a: unchecked address (e.g., 1st arg to mmap)
 # b: non-NULL buffer (e.g., 2nd arg to read; return value from mmap)
 # B: optionally-NULL buffer (e.g., 4th arg to getsockopt)
+# C: pthread cancellation endpoint
 # f: buffer of 2 ints (e.g., 4th arg to socketpair)
 # F: 3rd arg to fcntl
 # i: scalar (any signedness & size: int, long, long long, enum, whatever)
--- sysdeps/unix/sysv/linux/syscalls.list
+++ sysdeps/unix/sysv/linux/syscalls.list
@@ -11,7 +11,7 @@
 epoll_ctl	EXTRA	epoll_ctl	i:iiip	epoll_ctl
 epoll_wait	EXTRA	epoll_wait	Ci:ipii	epoll_wait
 epoll_pwait	EXTRA	epoll_pwait	Ci:ipiipi	epoll_pwait
-fdatasync	-	fdatasync	i:i	fdatasync
+fdatasync	-	fdatasync	Ci:i	fdatasync
 flock		-	flock		i:ii	__flock		flock
 fork		-	fork		i:	__libc_fork	__fork fork
 get_kernel_syms	EXTRA	get_kernel_syms	i:p	get_kernel_syms
--- io/lockf.c
+++ io/lockf.c
@@ -18,6 +18,7 @@
 
 #include <sys/types.h>
 #include <unistd.h>
+#include <sysdep-cancel.h>	/* Must come before <fcntl.h>.  */
 #include <fcntl.h>
 #include <errno.h>
 #include <string.h>
@@ -68,7 +69,16 @@
     }
 
   /* lockf() is a cancellation point but so is fcntl() if F_SETLKW is
-     used.  Therefore we don't have to care about cancellation here,
-     the fcntl() function will take care of it.  */
-  return __fcntl (fd, cmd, &fl);
+     used.  Therefore we don't have to care about cancellation in that
+     case as the fcntl() function will take care of it.  */
+  if (SINGLE_THREAD_P || cmd == F_SETLKW)
+    return __fcntl (fd, cmd, &fl);
+
+  int oldtype = LIBC_CANCEL_ASYNC ();
+
+  int result = __fcntl (fd, cmd, &fl);
+
+  LIBC_CANCEL_RESET (oldtype);
+
+  return result;
 }
--- nptl/tst-cancel-wrappers.sh
+++ nptl/tst-cancel-wrappers.sh
@@ -26,7 +26,9 @@ C["close"]=1
 C["connect"]=1
 C["creat"]=1
 C["fcntl"]=1
+C["fdatasync"]=1
 C["fsync"]=1
+C["lockf"]=1
 C["msgrcv"]=1
 C["msgsnd"]=1
 C["msync"]=1
--- nptl/tst-cancel4.c
+++ nptl/tst-cancel4.c
@@ -1571,6 +1571,47 @@ tf_fsync (void *arg)
 
 
 static void *
+tf_fdatasync (void *arg)
+{
+  if (arg == NULL)
+    // XXX If somebody can provide a portable test case in which fdatasync()
+    // blocks we can enable this test to run in both rounds.
+    abort ();
+
+  tempfd = open ("Makefile", O_RDONLY);
+  if (tempfd == -1)
+    {
+      printf ("%s: cannot open Makefile\n", __FUNCTION__);
+      exit (1);
+    }
+
+  int r = pthread_barrier_wait (&b2);
+  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: barrier_wait failed\n", __FUNCTION__);
+      exit (1);
+    }
+
+  r = pthread_barrier_wait (&b2);
+  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
+    {
+      printf ("%s: 2nd barrier_wait failed\n", __FUNCTION__);
+      exit (1);
+    }
+
+  pthread_cleanup_push (cl, NULL);
+
+  fdatasync (tempfd);
+
+  pthread_cleanup_pop (0);
+
+  printf ("%s: fdatasync returned\n", __FUNCTION__);
+
+  exit (1);
+}
+
+
+static void *
 tf_msync (void *arg)
 {
   if (arg == NULL)
@@ -2078,6 +2119,7 @@ static struct
   ADD_TEST (pread, 2, 1),
   ADD_TEST (pwrite, 2, 1),
   ADD_TEST (fsync, 2, 1),
+  ADD_TEST (fdatasync, 2, 1),
   ADD_TEST (msync, 2, 1),
   ADD_TEST (sendto, 2, 1),
   ADD_TEST (sendmsg, 2, 1),