diff options
author | Ryan Prichard <rprichard@google.com> | 2019-05-07 21:21:08 +0000 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2019-05-07 21:21:08 +0000 |
commit | 2a24f6bd5f61ee4e916f08a7a69b486891f12f44 (patch) | |
tree | 00d8377b3273af0bb9b07e858340673ac5619a82 | |
parent | bffe6f16a81d35b54a8c746fa5d3f8bec58c6a87 (diff) | |
parent | c8e263becfa01afecba84edf6a8a18da27a78e89 (diff) | |
download | android_bionic-2a24f6bd5f61ee4e916f08a7a69b486891f12f44.tar.gz android_bionic-2a24f6bd5f61ee4e916f08a7a69b486891f12f44.tar.bz2 android_bionic-2a24f6bd5f61ee4e916f08a7a69b486891f12f44.zip |
Merge "Revert fwalk/sfp locking to fix concurrent reads" into qt-dev
-rw-r--r-- | libc/stdio/local.h | 1 | ||||
-rw-r--r-- | libc/stdio/refill.c | 2 | ||||
-rw-r--r-- | libc/stdio/stdio.cpp | 17 | ||||
-rw-r--r-- | libc/stdlib/atexit.c | 3 | ||||
-rw-r--r-- | tests/stdio_test.cpp | 22 |
5 files changed, 38 insertions, 7 deletions
diff --git a/libc/stdio/local.h b/libc/stdio/local.h index d306a2130..1ecf1220f 100644 --- a/libc/stdio/local.h +++ b/libc/stdio/local.h @@ -204,6 +204,7 @@ __LIBC32_LEGACY_PUBLIC__ int __sclose(void*); __LIBC32_LEGACY_PUBLIC__ int _fwalk(int (*)(FILE*)); off64_t __sseek64(void*, off64_t, int); +int __sflush_locked(FILE*); int __swhatbuf(FILE*, size_t*, int*); wint_t __fgetwc_unlock(FILE*); wint_t __ungetwc(wint_t, FILE*); diff --git a/libc/stdio/refill.c b/libc/stdio/refill.c index cfa2bfd45..1df419180 100644 --- a/libc/stdio/refill.c +++ b/libc/stdio/refill.c @@ -40,7 +40,7 @@ static int lflush(FILE *fp) { if ((fp->_flags & (__SLBF|__SWR)) == (__SLBF|__SWR)) - return (__sflush(fp)); /* ignored... */ + return (__sflush_locked(fp)); /* ignored... */ return (0); } diff --git a/libc/stdio/stdio.cpp b/libc/stdio/stdio.cpp index 4cec757e5..91c7689f8 100644 --- a/libc/stdio/stdio.cpp +++ b/libc/stdio/stdio.cpp @@ -106,7 +106,7 @@ FILE* stdin = &__sF[0]; FILE* stdout = &__sF[1]; FILE* stderr = &__sF[2]; -static pthread_mutex_t __stdio_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP; +static pthread_mutex_t __stdio_mutex = PTHREAD_MUTEX_INITIALIZER; static uint64_t __get_file_tag(FILE* fp) { // Don't use a tag for the standard streams. @@ -211,21 +211,23 @@ found: } int _fwalk(int (*callback)(FILE*)) { - pthread_mutex_lock(&__stdio_mutex); int result = 0; for (glue* g = &__sglue; g != nullptr; g = g->next) { FILE* fp = g->iobs; for (int n = g->niobs; --n >= 0; ++fp) { - ScopedFileLock sfl(fp); if (fp->_flags != 0 && (fp->_flags & __SIGN) == 0) { result |= (*callback)(fp); } } } - pthread_mutex_unlock(&__stdio_mutex); return result; } +extern "C" __LIBC_HIDDEN__ void __libc_stdio_cleanup(void) { + // Equivalent to fflush(nullptr), but without all the locking since we're shutting down anyway. + _fwalk(__sflush); +} + static FILE* __fopen(int fd, int flags) { #if !defined(__LP64__) if (fd > SHRT_MAX) { @@ -520,6 +522,11 @@ int __sflush(FILE* fp) { return 0; } +int __sflush_locked(FILE* fp) { + ScopedFileLock sfl(fp); + return __sflush(fp); +} + int __sread(void* cookie, char* buf, int n) { FILE* fp = reinterpret_cast<FILE*>(cookie); return TEMP_FAILURE_RETRY(read(fp->_file, buf, n)); @@ -1061,7 +1068,7 @@ int wscanf(const wchar_t* fmt, ...) { } static int fflush_all() { - return _fwalk(__sflush); + return _fwalk(__sflush_locked); } int fflush(FILE* fp) { diff --git a/libc/stdlib/atexit.c b/libc/stdlib/atexit.c index 692e0c0fc..0efb11854 100644 --- a/libc/stdlib/atexit.c +++ b/libc/stdlib/atexit.c @@ -188,7 +188,8 @@ restart: /* If called via exit(), flush output of all open files. */ if (dso == NULL) { - fflush(NULL); + extern void __libc_stdio_cleanup(void); + __libc_stdio_cleanup(); } /* BEGIN android-changed: call __unregister_atfork if dso is not null */ diff --git a/tests/stdio_test.cpp b/tests/stdio_test.cpp index 65a942c37..c237d6d32 100644 --- a/tests/stdio_test.cpp +++ b/tests/stdio_test.cpp @@ -29,6 +29,7 @@ #include <locale.h> #include <string> +#include <thread> #include <vector> #include <android-base/file.h> @@ -2577,3 +2578,24 @@ TEST(STDIO_TEST, dev_std_files) { ASSERT_LT(0, length); ASSERT_EQ("/proc/self/fd/2", std::string(path, length)); } + +TEST(STDIO_TEST, fread_with_locked_file) { + // Reading an unbuffered/line-buffered file from one thread shouldn't block on + // files locked on other threads, even if it flushes some line-buffered files. + FILE* fp1 = fopen("/dev/zero", "r"); + ASSERT_TRUE(fp1 != nullptr); + flockfile(fp1); + + std::thread([] { + for (int mode : { _IONBF, _IOLBF }) { + FILE* fp2 = fopen("/dev/zero", "r"); + ASSERT_TRUE(fp2 != nullptr); + setvbuf(fp2, nullptr, mode, 0); + ASSERT_EQ('\0', fgetc(fp2)); + fclose(fp2); + } + }).join(); + + funlockfile(fp1); + fclose(fp1); +} |