diff options
Diffstat (limited to 'ipc/chromium')
-rw-r--r-- | ipc/chromium/src/base/process_util_posix.cc | 78 | ||||
-rw-r--r-- | ipc/chromium/src/chrome/common/ipc_channel.h | 9 | ||||
-rw-r--r-- | ipc/chromium/src/chrome/common/ipc_channel_posix.cc | 8 | ||||
-rw-r--r-- | ipc/chromium/src/mojo/core/ports/port.cc | 4 | ||||
-rw-r--r-- | ipc/chromium/src/mojo/core/ports/port.h | 36 | ||||
-rw-r--r-- | ipc/chromium/src/mojo/core/ports/port_locker.cc | 8 |
6 files changed, 130 insertions, 13 deletions
diff --git a/ipc/chromium/src/base/process_util_posix.cc b/ipc/chromium/src/base/process_util_posix.cc index d91dc25e9f..57ae82ebbf 100644 --- a/ipc/chromium/src/base/process_util_posix.cc +++ b/ipc/chromium/src/base/process_util_posix.cc @@ -10,6 +10,7 @@ #include <signal.h> #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <sys/resource.h> #include <sys/time.h> #include <sys/types.h> @@ -37,6 +38,7 @@ #ifdef MOZ_ENABLE_FORKSERVER # include "mozilla/ipc/ForkServiceChild.h" +# include "mozilla/Printf.h" #endif // We could configure-test for `waitid`, but it's been in POSIX for a @@ -195,6 +197,63 @@ void CloseSuperfluousFds(void* aCtx, bool (*aShouldPreserve)(void*, int)) { } } +#ifdef MOZ_ENABLE_FORKSERVER +// Returns whether a process (assumed to still exist) is in the zombie +// state. Any failures (if the process doesn't exist, if /proc isn't +// mounted, etc.) will return true, so that we don't try again. +static bool IsZombieProcess(pid_t pid) { +# ifdef XP_LINUX + auto path = mozilla::Smprintf("/proc/%d/stat", pid); + int fd = open(path.get(), O_RDONLY | O_CLOEXEC); + if (fd < 0) { + int e = errno; + CHROMIUM_LOG(ERROR) << "failed to open " << path.get() << ": " + << strerror(e); + return true; + } + + // /proc/%d/stat format is approximately: + // + // %d (%s) %c %d %d %d %d %d ... + // + // The state is the third field; the second field is the thread + // name, in parentheses, but it can contain arbitrary characters. + // So, we read the whole line, check for the last ')' because all of + // the following fields are numeric, and move forward from there. + // + // And because (unlike other uses of this info the codebase) we + // don't care about those other fields, we can read a smaller amount + // of the file. + + char buffer[64]; + ssize_t len = HANDLE_EINTR(read(fd, buffer, sizeof(buffer) - 1)); + int e = errno; + close(fd); + if (len < 1) { + CHROMIUM_LOG(ERROR) << "failed to read " << buffer << ": " << strerror(e); + return true; + } + + buffer[len] = '\0'; + char* rparen = strrchr(buffer, ')'); + if (!rparen || rparen[1] != ' ' || rparen[2] == '\0') { + DCHECK(false) << "/proc/{pid}/stat parse error"; + CHROMIUM_LOG(ERROR) << "bad data in /proc/" << pid << "/stat"; + return true; + } + if (rparen[2] == 'Z') { + CHROMIUM_LOG(ERROR) << "process " << pid << " is a zombie"; + return true; + } + return false; +# else // not XP_LINUX + // The situation where this matters is Linux-specific (pid + // namespaces), so we don't need to bother on other Unixes. + return false; +# endif +} +#endif // MOZ_ENABLE_FORKSERVER + bool IsProcessDead(ProcessHandle handle, bool blocking) { auto handleForkServer = [handle]() -> mozilla::Maybe<bool> { #ifdef MOZ_ENABLE_FORKSERVER @@ -205,10 +264,21 @@ bool IsProcessDead(ProcessHandle handle, bool blocking) { // process any more, it is impossible to use |waitpid()| to wait for // them. const int r = kill(handle, 0); - // FIXME: for unexpected errors we should probably log a warning - // and return true, so that the caller doesn't loop / hang / - // try to kill the process. (Bug 1658072 will rewrite this code.) - return mozilla::Some(r < 0 && errno == ESRCH); + if (r < 0) { + const int e = errno; + if (e != ESRCH) { + CHROMIUM_LOG(WARNING) << "unexpected error checking for process " + << handle << ": " << strerror(e); + // Return true for unknown errors, to avoid the possibility + // of getting stuck in loop of failures. + } + return mozilla::Some(true); + } + // Annoying edge case (bug NNNNNNN): if pid 1 isn't a real + // `init`, like in some container environments, and if the child + // exited after the fork server, it could become a permanent + // zombie. We treat it as dead in that case. + return mozilla::Some(IsZombieProcess(handle)); } #else mozilla::Unused << handle; diff --git a/ipc/chromium/src/chrome/common/ipc_channel.h b/ipc/chromium/src/chrome/common/ipc_channel.h index 5d342d7252..d755810078 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel.h +++ b/ipc/chromium/src/chrome/common/ipc_channel.h @@ -142,11 +142,12 @@ class Channel { void StartAcceptingHandles(Mode mode); #endif -#if defined(MOZ_WIDGET_ANDROID) - // Used to set the first IPC file descriptor in the child process on Android. - // See ipc_channel_posix.cc for further details on how this is used. +#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_UIKIT) + // Used to set the first IPC file descriptor in the child process on + // Android and iOS. See ipc_channel_posix.cc for further details on how this + // is used. static void SetClientChannelFd(int fd); -#endif // defined(MOZ_WIDGET_ANDROID) +#endif // defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_UIKIT) // Get the first IPC channel handle in the child process. This will have been // set by SetClientChannelFd on Android, will be a constant on other unix diff --git a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc index 0b34af40b1..19e777a52a 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc +++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc @@ -84,8 +84,8 @@ namespace { // This is the file descriptor number that a client process expects to find its // IPC socket. static int gClientChannelFd = -#if defined(MOZ_WIDGET_ANDROID) - // On android the fd is set at the time of child creation. +#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_UIKIT) + // On android/ios the fd is set at the time of child creation. -1 #else 3 @@ -135,9 +135,9 @@ static inline ssize_t corrected_sendmsg(int socket, } // namespace //------------------------------------------------------------------------------ -#if defined(MOZ_WIDGET_ANDROID) +#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_UIKIT) void Channel::SetClientChannelFd(int fd) { gClientChannelFd = fd; } -#endif // defined(MOZ_WIDGET_ANDROID) +#endif // defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_UIKIT) int Channel::GetClientChannelHandle() { return gClientChannelFd; } diff --git a/ipc/chromium/src/mojo/core/ports/port.cc b/ipc/chromium/src/mojo/core/ports/port.cc index 871ec8fca6..df46ae100d 100644 --- a/ipc/chromium/src/mojo/core/ports/port.cc +++ b/ipc/chromium/src/mojo/core/ports/port.cc @@ -8,6 +8,10 @@ namespace mojo { namespace core { namespace ports { +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX +mozilla::StaticMutex detail::PortMutex::sSingleton; +#endif + // Used by std::{push,pop}_heap functions inline bool operator<(const mozilla::UniquePtr<Event>& a, const mozilla::UniquePtr<Event>& b) { diff --git a/ipc/chromium/src/mojo/core/ports/port.h b/ipc/chromium/src/mojo/core/ports/port.h index 44529ddb6f..78fe0109b8 100644 --- a/ipc/chromium/src/mojo/core/ports/port.h +++ b/ipc/chromium/src/mojo/core/ports/port.h @@ -18,6 +18,18 @@ #include "mozilla/RefPtr.h" #include "nsISupportsImpl.h" +#ifdef MOZ_TSAN +// In TSAN builds, a single static mutex is used for all ports, rather than +// per-port mutexes, to avoid overloading the maximum 64 concurrently-held locks +// limit of its deadlock detector when sending messages with a large number of +// attached ports. +# define MOZ_USE_SINGLETON_PORT_MUTEX 1 +#endif + +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX +# include "mozilla/StaticMutex.h" +#endif + namespace mojo { namespace core { namespace ports { @@ -28,20 +40,38 @@ namespace detail { // Ports cannot use mozilla::Mutex, as the acquires-before relationships handled // by PortLocker can overload the debug-only deadlock detector. -class MOZ_CAPABILITY("mutex") PortMutex : private ::mozilla::detail::MutexImpl { +class MOZ_CAPABILITY("mutex") PortMutex +#ifndef MOZ_USE_SINGLETON_PORT_MUTEX + : private ::mozilla::detail::MutexImpl +#endif +{ public: void AssertCurrentThreadOwns() const MOZ_ASSERT_CAPABILITY(this) { #ifdef DEBUG MOZ_ASSERT(mOwningThread == PR_GetCurrentThread()); #endif +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + sSingleton.AssertCurrentThreadOwns(); +#endif } private: // PortMutex should only be locked/unlocked via PortLocker friend class ::mojo::core::ports::PortLocker; +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + // If the singleton mutex is in use, it must be locked before calling `Lock()` + // on any port, and must only be unlocked after calling `Unlock()` on every + // locked port. + static ::mozilla::StaticMutex sSingleton; +#endif + void Lock() MOZ_CAPABILITY_ACQUIRE() { +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + sSingleton.AssertCurrentThreadOwns(); +#else ::mozilla::detail::MutexImpl::lock(); +#endif #ifdef DEBUG mOwningThread = PR_GetCurrentThread(); #endif @@ -51,7 +81,11 @@ class MOZ_CAPABILITY("mutex") PortMutex : private ::mozilla::detail::MutexImpl { MOZ_ASSERT(mOwningThread == PR_GetCurrentThread()); mOwningThread = nullptr; #endif +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + sSingleton.AssertCurrentThreadOwns(); +#else ::mozilla::detail::MutexImpl::unlock(); +#endif } #ifdef DEBUG diff --git a/ipc/chromium/src/mojo/core/ports/port_locker.cc b/ipc/chromium/src/mojo/core/ports/port_locker.cc index 044a26f143..a09bed31c3 100644 --- a/ipc/chromium/src/mojo/core/ports/port_locker.cc +++ b/ipc/chromium/src/mojo/core/ports/port_locker.cc @@ -36,6 +36,10 @@ PortLocker::PortLocker(const PortRef** port_refs, size_t num_ports) UpdateTLS(nullptr, this); #endif +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + detail::PortMutex::sSingleton.Lock(); +#endif + // Sort the ports by address to lock them in a globally consistent order. std::sort( port_refs_, port_refs_ + num_ports_, @@ -52,6 +56,10 @@ PortLocker::~PortLocker() { port_refs_[i]->port()->lock_.Unlock(); } +#ifdef MOZ_USE_SINGLETON_PORT_MUTEX + detail::PortMutex::sSingleton.Unlock(); +#endif + #ifdef DEBUG UpdateTLS(this, nullptr); #endif |