This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] [BZ #22830] malloc_stats: restore cancellation for stderr correctly.
- From: Zack Weinberg <zackw at panix dot com>
- To: libc-alpha at sourceware dot org
- Date: Sat, 10 Feb 2018 16:22:05 -0500
- Subject: [PATCH] [BZ #22830] malloc_stats: restore cancellation for stderr correctly.
- Authentication-results: sourceware.org; auth=none
malloc_stats means to disable cancellation for writes to stderr while
it runs, but it restores stderr->_flags2 with |= instead of =, so what
it actually does is disable cancellation on stderr permanently.
I'm going to go ahead and commit this as an obvious bugfix, but I
would welcome feedback on the test case, which is rather complicated
and could probably be made more robust (for instance, setting stderr to
line-buffered breaks the test and I don't understand why).
Perhaps _IO_lock_acquire_clear_flags2 should be extended to handle
_IO_FLAGS2_NOTCANCEL as well as what it already does, but that would
_not_ be an obvious bugfix. :-)
[BZ #22830]
* malloc/malloc.c (__malloc_stats): Restore stderr->_flags2
correctly.
* malloc/tst-malloc-stats-cancellation.c: New test case.
* malloc/Makefile: Add new test case.
---
malloc/Makefile | 2 +
malloc/malloc.c | 2 +-
malloc/tst-malloc-stats-cancellation.c | 216 +++++++++++++++++++++++++++++++++
3 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 malloc/tst-malloc-stats-cancellation.c
diff --git a/malloc/Makefile b/malloc/Makefile
index 17873e67c4b..7d54bad866f 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -37,6 +37,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
tst-malloc-tcache-leak \
tst-malloc_info \
tst-malloc-too-large \
+ tst-malloc-stats-cancellation \
tests-static := \
tst-interpose-static-nothread \
@@ -96,6 +97,7 @@ $(objpfx)tst-malloc-backtrace: $(shared-thread-library)
$(objpfx)tst-malloc-thread-exit: $(shared-thread-library)
$(objpfx)tst-malloc-thread-fail: $(shared-thread-library)
$(objpfx)tst-malloc-fork-deadlock: $(shared-thread-library)
+$(objpfx)tst-malloc-stats-cancellation: $(shared-thread-library)
# Export the __malloc_initialize_hook variable to libc.so.
LDFLAGS-tst-mallocstate = -rdynamic
diff --git a/malloc/malloc.c b/malloc/malloc.c
index f8e7250f70f..3224b3a899f 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5009,7 +5009,7 @@ __malloc_stats (void)
fprintf (stderr, "max mmap regions = %10u\n", (unsigned int) mp_.max_n_mmaps);
fprintf (stderr, "max mmap bytes = %10lu\n",
(unsigned long) mp_.max_mmapped_mem);
- ((_IO_FILE *) stderr)->_flags2 |= old_flags2;
+ ((_IO_FILE *) stderr)->_flags2 = old_flags2;
_IO_funlockfile (stderr);
}
diff --git a/malloc/tst-malloc-stats-cancellation.c b/malloc/tst-malloc-stats-cancellation.c
new file mode 100644
index 00000000000..9a8f475830b
--- /dev/null
+++ b/malloc/tst-malloc-stats-cancellation.c
@@ -0,0 +1,216 @@
+/* Bug 22830: malloc_stats fails to re-enable cancellation on exit.
+ Copyright (C) 2018 Free Software Foundation.
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. This file is offered as-is,
+ without any warranty. */
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <pthread.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <malloc.h>
+
+static void *
+test_threadproc (void *gatep)
+{
+ /* When we are released from the barrier, there is a cancellation
+ request pending for this thread. N.B. pthread_barrier_wait is
+ not itself a cancellation point (oddly enough). */
+ pthread_barrier_wait ((pthread_barrier_t *)gatep);
+ malloc_stats ();
+ fputs ("this call should trigger cancellation\n", stderr);
+ return 0;
+}
+
+/* We cannot replace stderr with a memstream because writes to memstreams
+ do not trigger cancellation. Instead, main swaps out fd 2 to point to
+ a pipe, and this thread reads from the pipe and writes to a memstream
+ until EOF, then returns the data accumulated in the memstream. main
+ can't do that itself because, when the test thread gets cancelled,
+ it doesn't close the pipe. */
+
+struct buffer_tp_args
+{
+ int ifd;
+ FILE *real_stderr;
+};
+
+static void *
+buffer_threadproc (void *argp)
+{
+ struct buffer_tp_args *args = argp;
+ int ifd = args->ifd;
+ char block[BUFSIZ], *p;
+ ssize_t nread;
+ size_t nwritten;
+
+ char *obuf = 0;
+ size_t obufsz = 0;
+ FILE *ofp = open_memstream (&obuf, &obufsz);
+ if (!ofp)
+ {
+ fprintf (args->real_stderr,
+ "buffer_threadproc: open_memstream: %s\n", strerror (errno));
+ return 0;
+ }
+
+ while ((nread = read (ifd, block, BUFSIZ)) > 0)
+ {
+ p = block;
+ do
+ {
+ nwritten = fwrite_unlocked (p, 1, nread, ofp);
+ if (nwritten == 0)
+ {
+ fprintf (args->real_stderr,
+ "buffer_threadproc: fwrite_unlocked: %s\n",
+ strerror (errno));
+ return 0;
+ }
+ nread -= nwritten;
+ p += nwritten;
+ }
+ while (nread > 0);
+ }
+ if (nread == -1)
+ {
+ fprintf (args->real_stderr, "buffer_threadproc: read: %s\n",
+ strerror (errno));
+ return 0;
+ }
+ close (ifd);
+ fclose (ofp);
+ return obuf;
+}
+
+
+int
+main (void)
+{
+ int result = 0, err, real_stderr_fd, bufpipe[2];
+ pthread_t t_thr, b_thr;
+ pthread_barrier_t gate;
+ void *rv;
+ FILE *real_stderr;
+ char *obuf;
+ void *obuf_v;
+ struct buffer_tp_args b_args;
+
+ real_stderr_fd = dup (2);
+ if (real_stderr_fd == -1)
+ {
+ perror ("dup");
+ return 2;
+ }
+ real_stderr = fdopen(real_stderr_fd, "w");
+ if (!real_stderr)
+ {
+ perror ("fdopen");
+ return 2;
+ }
+ if (setvbuf (real_stderr, 0, _IOLBF, 0))
+ {
+ perror ("setvbuf(real_stderr)");
+ return 2;
+ }
+
+ if (pipe (bufpipe))
+ {
+ perror ("pipe");
+ return 2;
+ }
+
+ /* Below this point, nobody other than the test_threadproc should use
+ the normal stderr. */
+ if (dup2 (bufpipe[1], 2) == -1)
+ {
+ fprintf (real_stderr, "dup2: %s\n", strerror (errno));
+ return 2;
+ }
+ close (bufpipe[1]);
+
+ b_args.ifd = bufpipe[0];
+ b_args.real_stderr = real_stderr;
+ err = pthread_create (&b_thr, 0, buffer_threadproc, &b_args);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_create(buffer_thr): %s\n",
+ strerror (err));
+ return 2;
+ }
+
+ err = pthread_barrier_init (&gate, 0, 2);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_barrier_init: %s\n", strerror (err));
+ return 2;
+ }
+
+ err = pthread_create (&t_thr, 0, test_threadproc, &gate);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_create(test_thr): %s\n", strerror (err));
+ return 2;
+ }
+
+ err = pthread_cancel (t_thr);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_cancel: %s\n", strerror (err));
+ return 2;
+ }
+
+ pthread_barrier_wait (&gate); /* cannot fail */
+
+ err = pthread_join (t_thr, &rv);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_join(test_thr): %s\n", strerror (err));
+ return 2;
+ }
+
+ /* Closing the normal stderr releases the buffer_threadproc from its
+ loop. */
+ fclose (stderr);
+ err = pthread_join (b_thr, &obuf_v);
+ if (err)
+ {
+ fprintf (real_stderr, "pthread_join(buffer_thr): %s\n", strerror (err));
+ return 2;
+ }
+ obuf = obuf_v;
+ if (obuf == 0)
+ return 2; /* error within buffer_threadproc, already reported */
+
+ if (rv != PTHREAD_CANCELED)
+ {
+ fputs ("FAIL: thread was not cancelled\n", real_stderr);
+ result = 1;
+ }
+ /* obuf should have received all of the text printed by malloc_stats,
+ but not the text printed by the final call to fputs. */
+ if (!strstr (obuf, "max mmap bytes"))
+ {
+ fputs ("FAIL: malloc_stats output incomplete\n", real_stderr);
+ result = 1;
+ }
+ if (strstr (obuf, "this call should trigger cancellation"))
+ {
+ fputs ("FAIL: fputs produced output\n", real_stderr);
+ result = 1;
+ }
+
+ if (result == 1)
+ {
+ fputs ("--- output from thread below ---\n", real_stderr);
+ fputs (obuf, real_stderr);
+ }
+ return result;
+}
--
2.15.1