Commit 2f74c00e authored by Konstantin Ganenko's avatar Konstantin Ganenko Committed by Commit Bot

Fix net services unittests on masos earlier than 10.13

The short crash stack:
6   libsystem_c.dylib                   0x00007fffb1a889ed reallocf + 21
7   libsystem_notify.dylib              0x00007fffb1bf962f notify_retain_file_descriptor + 160
8   libsystem_notify.dylib              0x00007fffb1bf94c3 notify_register_file_descriptor + 1091
9   libnet.dylib                        0x0000000111832021 net::NotifyWatcherMac::Watch(char const*, base::RepeatingCallback<void (bool)> const&) + 241

Crash happens on OS X 10.12.X.
The fall occurs in the libnotify library.
For macos 10.12.X libnotify library version 165 is used.
https://opensource.apple.com/release/macos-1012.html
By source code:
https://opensource.apple.com/source/Libnotify/Libnotify-165/notify_client.c.auto.html
the fall occurs in

globals->fd_clnt = (int *)reallocf(globals - >fd_clnt, globals->fd_count * sizeof(int));

, or the same with fd_srv, fd_refcount.
Global pointers to file descriptor tables are reset only when global is initialized and nowhere else.
On releasing the last file descriptor free is called  for these pointers without zeroing them. (see notify_release_file_descriptor in same file).
Accordingly, when creating one file descriptor (notify_register_file_descriptor)  with subsequent its destruction (notify_cancel), the creation of the next will lead to a crash - realloc on freed pointer.
Here introduced simple approach to repair issue. We need to hold one stub file descriptor to avoid freeing globals in libnotify.
For earlier implementations of the libraries (looked 10.11.X - version of library is 149) the problem is also there.

More detailed crash stack is added in issue.

R=agl@chromium.org

Bug: 783148
Change-Id: I23396372bf6fdff78c70bf6a53a4183795677b02
Reviewed-on: https://chromium-review.googlesource.com/1168490
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582612}
parent f16a864a
......@@ -7,13 +7,38 @@
#include <notify.h>
#include "base/logging.h"
#include "base/mac/mac_util.h"
#include "base/message_loop/message_loop_current.h"
#include "base/posix/eintr_wrapper.h"
namespace net {
namespace {
// Registers a dummy file descriptor to workaround a bug in libnotify
// in macOS 10.12
// See https://bugs.chromium.org/p/chromium/issues/detail?id=783148.
class NotifyFileDescriptorsGlobalsHolder {
public:
NotifyFileDescriptorsGlobalsHolder() {
int notify_fd = -1;
int notify_token = -1;
notify_register_file_descriptor("notify_file_descriptor_holder", &notify_fd,
0, &notify_token);
}
};
void HoldNotifyFileDescriptorsGlobals() {
if (base::mac::IsAtMostOS10_12()) {
static NotifyFileDescriptorsGlobalsHolder holder;
}
}
} // namespace
NotifyWatcherMac::NotifyWatcherMac()
: notify_fd_(-1), notify_token_(-1), watcher_(FROM_HERE) {}
: notify_fd_(-1), notify_token_(-1), watcher_(FROM_HERE) {
HoldNotifyFileDescriptorsGlobals();
}
NotifyWatcherMac::~NotifyWatcherMac() {
Cancel();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment