tst-cancel4: Make blocking on write more portable
Commit Message
We should not be assuming how PF_UNIX buffering works and just choose a
"proper" write size. The additional pthread_testcancel() call after
write() shows it: we don't really control how buffering works.
We can however easily use select to fill the buffer byte by byte until
select() says that write() will block, in which case write() will not
even be able to perform a partial write.
We should however do this on the same side of the socketpair as the
read() tests, otherwise read() would get data from this. This is
actually fine: we can just read() and write() on the same side, with
nobody consuming or producing data on the other side.
* nptl/tst-cancel4-common.h (set_socket_buffer): Write data on the
socket until select informs us that it would block.
* nptl/tst-cancel4-common.c (do_test): Set send buffer size on fds[0]
instead of fds[1].
* nptl/tst-cancel4.c (tf_write, tf_writev): Use fds[0] instead of
fds[1].
(tf_write): Do not call pthread_testcancel() after write(), we are now not
even supposed to get partial writes.
(tf_send, tf_sendto): Fill socket buffer after connecting.
---
nptl/tst-cancel4-common.c | 2 +-
nptl/tst-cancel4-common.h | 19 ++++++++++++++++++-
nptl/tst-cancel4.c | 16 ++++++----------
3 files changed, 25 insertions(+), 12 deletions(-)
Comments
On 05/07/2020 15:11, Samuel Thibault wrote:
> diff --git a/nptl/tst-cancel4-common.h b/nptl/tst-cancel4-common.h
> index c8763cacba..f99785c4f2 100644
> --- a/nptl/tst-cancel4-common.h
> +++ b/nptl/tst-cancel4-common.h
> @@ -62,7 +62,7 @@ static pthread_barrier_t b2;
>
> #define WRITE_BUFFER_SIZE 16384
>
> -/* Set the send buffer of socket S to 1 byte so any send operation
> +/* Set the send buffer of socket S to 1 byte and fill it so any send operation
> done with WRITE_BUFFER_SIZE bytes will force syscall blocking. */
> static void
> set_socket_buffer (int s)
> @@ -74,6 +74,23 @@ set_socket_buffer (int s)
> sizeof (val)) == 0);
> TEST_VERIFY_EXIT (getsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, &len) == 0);
> TEST_VERIFY_EXIT (val < WRITE_BUFFER_SIZE);
> +
> + char buf = '\0';
> + struct timeval timeout = { .tv_sec = 0, .tv_usec = 0 };
> +
> + while (1)
> + {
> + fd_set set;
> + int ret;
> +
> + FD_ZERO (&set);
> + FD_SET (s, &set);
> + ret = select (s + 1, NULL, &set, NULL, &timeout);
> + if (!ret)
> + break;
> +
> + write (s, &buf, 1);
> + }
> }
Unfortunately this does not do what you are intending to do on Linux and
the tests only pass because of BZ#12683 where cancellation does not act on
write with side-effects.
For instance on kernel 5.4.0-40-generic the syscall sequence is:
[...]
select(4, NULL, [3], NULL, {tv_sec=0, tv_usec=0}) = 1 (out [3], left {tv_sec=0, tv_usec=0})
write(3, "\0", 1) = 1
select(4, NULL, [3], NULL, {tv_sec=0, tv_usec=0}) = 1 (out [3], left {tv_sec=0, tv_usec=0})
write(3, "\0", 1) = 1
select(4, NULL, [3], NULL, {tv_sec=0, tv_usec=0}) = 0 (Timeout)
[...]
clone(child_stack=0x7fd3ee4f4fb0, flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTIDstrace: Process 2641554 attached
, parent_tid=[2641554], tls=0x7fd3ee4f5700, child_tidptr=0x7fd3ee4f59d0) = 2641554
[...]
[pid 2641554] write(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384 <unfinished ...>
[...]
[pid 2641553] tgkill(2641553, 2641554, SIGRTMIN) = 0
[pid 2641554] write(1, "write (3, buf, 16384) returned 2"..., 40write (3, buf, 16384) returned 2240 (0)
[...]
So it only fills 2 bytes on the socket buffer, but later the write *does*
return with side-effects indicating that it has at least written 2240 bytes.
I haven't investigate further on Linux kernel the iteration between
SOL_SOCKET, buffer filling and blocking operations. But it does show that
is indeed system specific and partial write might still occur even with
clever tricks. I am also not sure if this is something specific to
socketpair and if using a named socket might have different results.
Are you trying to make the tests behave better on Hurd? If so, in how
exactly Hurd behaves different than Linux in this case?
@@ -26,7 +26,7 @@ do_test (void)
exit (1);
}
- set_socket_buffer (fds[1]);
+ set_socket_buffer (fds[0]);
if (mktemp (fifoname) == NULL)
{
@@ -62,7 +62,7 @@ static pthread_barrier_t b2;
#define WRITE_BUFFER_SIZE 16384
-/* Set the send buffer of socket S to 1 byte so any send operation
+/* Set the send buffer of socket S to 1 byte and fill it so any send operation
done with WRITE_BUFFER_SIZE bytes will force syscall blocking. */
static void
set_socket_buffer (int s)
@@ -74,6 +74,23 @@ set_socket_buffer (int s)
sizeof (val)) == 0);
TEST_VERIFY_EXIT (getsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, &len) == 0);
TEST_VERIFY_EXIT (val < WRITE_BUFFER_SIZE);
+
+ char buf = '\0';
+ struct timeval timeout = { .tv_sec = 0, .tv_usec = 0 };
+
+ while (1)
+ {
+ fd_set set;
+ int ret;
+
+ FD_ZERO (&set);
+ FD_SET (s, &set);
+ ret = select (s + 1, NULL, &set, NULL, &timeout);
+ if (!ret)
+ break;
+
+ write (s, &buf, 1);
+ }
}
/* Cleanup handling test. */
@@ -147,7 +147,7 @@ tf_write (void *arg)
int fd;
if (arg == NULL)
- fd = fds[1];
+ fd = fds[0];
else
{
char fname[] = "/tmp/tst-cancel4-fd-XXXXXX";
@@ -167,10 +167,6 @@ tf_write (void *arg)
char buf[WRITE_BUFFER_SIZE];
memset (buf, '\0', sizeof (buf));
s = write (fd, buf, sizeof (buf));
- /* The write can return a value higher than 0 (meaning partial write)
- due to the SIGCANCEL, but the thread may still be pending
- cancellation. */
- pthread_testcancel ();
pthread_cleanup_pop (0);
@@ -184,7 +180,7 @@ tf_writev (void *arg)
int fd;
if (arg == NULL)
- fd = fds[1];
+ fd = fds[0];
else
{
char fname[] = "/tmp/tst-cancel4-fd-XXXXXX";
@@ -753,13 +749,13 @@ tf_send (void *arg)
if (tempfd2 == -1)
FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m");
- set_socket_buffer (tempfd2);
-
if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0)
FAIL_EXIT1 ("connect: %m");
unlink (sun.sun_path);
+ set_socket_buffer (tempfd2);
+
xpthread_barrier_wait (&b2);
if (arg != NULL)
@@ -1288,13 +1284,13 @@ tf_sendto (void *arg)
if (tempfd2 == -1)
FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m");
- set_socket_buffer (tempfd2);
-
if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0)
FAIL_EXIT1 ("connect: %m");
unlink (sun.sun_path);
+ set_socket_buffer (tempfd2);
+
xpthread_barrier_wait (&b2);
if (arg != NULL)