Share ScreenDrawerLockPosix implementation
This change renames ScreenDrawerLockLinux into ScreenDrawerLockPosix and shares
it with Mac OSX.
Bug: webrtc:7950
Change-Id: Ib141781d2c35bfda0d6f9458fff235adbb643280
Reviewed-on: https://chromium-review.googlesource.com/607688
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19393}diff --git a/webrtc/modules/desktop_capture/BUILD.gn b/webrtc/modules/desktop_capture/BUILD.gn
index caf27a3..92d670c 100644
--- a/webrtc/modules/desktop_capture/BUILD.gn
+++ b/webrtc/modules/desktop_capture/BUILD.gn
@@ -127,6 +127,8 @@
"screen_drawer.cc",
"screen_drawer.h",
"screen_drawer_linux.cc",
+ "screen_drawer_lock_posix.cc",
+ "screen_drawer_lock_posix.h",
"screen_drawer_mac.cc",
"screen_drawer_win.cc",
]
diff --git a/webrtc/modules/desktop_capture/screen_drawer_linux.cc b/webrtc/modules/desktop_capture/screen_drawer_linux.cc
index 9a9c20d..fa2cfa4 100644
--- a/webrtc/modules/desktop_capture/screen_drawer_linux.cc
+++ b/webrtc/modules/desktop_capture/screen_drawer_linux.cc
@@ -8,47 +8,22 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <semaphore.h>
#include <string.h>
#include <X11/Xlib.h>
#include <memory>
#include "webrtc/modules/desktop_capture/screen_drawer.h"
+#include "webrtc/modules/desktop_capture/screen_drawer_lock_posix.h"
#include "webrtc/modules/desktop_capture/x11/shared_x_display.h"
#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/ptr_util.h"
#include "webrtc/system_wrappers/include/sleep.h"
namespace webrtc {
namespace {
-static constexpr char kSemaphoreName[] =
- "/global-screen-drawer-linux-54fe5552-8047-11e6-a725-3f429a5b4fb4";
-
-class ScreenDrawerLockLinux : public ScreenDrawerLock {
- public:
- ScreenDrawerLockLinux();
- ~ScreenDrawerLockLinux() override;
-
- private:
- sem_t* semaphore_;
-};
-
-ScreenDrawerLockLinux::ScreenDrawerLockLinux() {
- semaphore_ =
- sem_open(kSemaphoreName, O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO, 1);
- sem_wait(semaphore_);
-}
-
-ScreenDrawerLockLinux::~ScreenDrawerLockLinux() {
- sem_post(semaphore_);
- sem_close(semaphore_);
- // sem_unlink(kSemaphoreName);
-}
-
// A ScreenDrawer implementation for X11.
class ScreenDrawerLinux : public ScreenDrawer {
public:
@@ -187,13 +162,13 @@
// static
std::unique_ptr<ScreenDrawerLock> ScreenDrawerLock::Create() {
- return std::unique_ptr<ScreenDrawerLock>(new ScreenDrawerLockLinux());
+ return rtc::MakeUnique<ScreenDrawerLockPosix>();
}
// static
std::unique_ptr<ScreenDrawer> ScreenDrawer::Create() {
if (SharedXDisplay::CreateDefault().get()) {
- return std::unique_ptr<ScreenDrawer>(new ScreenDrawerLinux());
+ return rtc::MakeUnique<ScreenDrawerLinux>();
}
return nullptr;
}
diff --git a/webrtc/modules/desktop_capture/screen_drawer_lock_posix.cc b/webrtc/modules/desktop_capture/screen_drawer_lock_posix.cc
new file mode 100644
index 0000000..5b6bc9c
--- /dev/null
+++ b/webrtc/modules/desktop_capture/screen_drawer_lock_posix.cc
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/modules/desktop_capture/screen_drawer_lock_posix.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/logging.h"
+
+namespace webrtc {
+
+namespace {
+
+// A uuid as the name of semaphore.
+static constexpr char kSemaphoreName[] = "GSDL54fe5552804711e6a7253f429a";
+
+} // namespace
+
+ScreenDrawerLockPosix::ScreenDrawerLockPosix()
+ : ScreenDrawerLockPosix(kSemaphoreName) {}
+
+ScreenDrawerLockPosix::ScreenDrawerLockPosix(const char* name) {
+ semaphore_ = sem_open(name, O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO, 1);
+ if (semaphore_ == SEM_FAILED) {
+ LOG_ERRNO(LS_ERROR) << "Failed to create named semaphore with " << name;
+ RTC_NOTREACHED();
+ }
+
+ sem_wait(semaphore_);
+}
+
+ScreenDrawerLockPosix::~ScreenDrawerLockPosix() {
+ if (semaphore_ == SEM_FAILED) {
+ return;
+ }
+
+ sem_post(semaphore_);
+ sem_close(semaphore_);
+ // sem_unlink a named semaphore won't wait until other clients to release the
+ // sem_t. So if a new process starts, it will sem_open a different kernel
+ // object with the same name and eventually breaks the cross-process lock.
+}
+
+// static
+void ScreenDrawerLockPosix::Unlink(const char* name) {
+ sem_unlink(name);
+}
+
+} // namespace webrtc
diff --git a/webrtc/modules/desktop_capture/screen_drawer_lock_posix.h b/webrtc/modules/desktop_capture/screen_drawer_lock_posix.h
new file mode 100644
index 0000000..a768066
--- /dev/null
+++ b/webrtc/modules/desktop_capture/screen_drawer_lock_posix.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_DESKTOP_CAPTURE_SCREEN_DRAWER_LOCK_POSIX_H_
+#define WEBRTC_MODULES_DESKTOP_CAPTURE_SCREEN_DRAWER_LOCK_POSIX_H_
+
+#include <semaphore.h>
+
+#include "webrtc/modules/desktop_capture/screen_drawer.h"
+
+namespace webrtc {
+
+class ScreenDrawerLockPosix final : public ScreenDrawerLock {
+ public:
+ ScreenDrawerLockPosix();
+ // Provides a name other than the default one for test only.
+ explicit ScreenDrawerLockPosix(const char* name);
+ ~ScreenDrawerLockPosix() override;
+
+ // Unlinks the named semaphore actively. This will remove the sem_t object in
+ // the system and allow others to create a different sem_t object with the
+ // same/ name.
+ static void Unlink(const char* name);
+
+ private:
+ sem_t* semaphore_;
+};
+
+} // namespace webrtc
+
+#endif // WEBRTC_MODULES_DESKTOP_CAPTURE_SCREEN_DRAWER_LOCK_POSIX_H_
diff --git a/webrtc/modules/desktop_capture/screen_drawer_mac.cc b/webrtc/modules/desktop_capture/screen_drawer_mac.cc
index c845929..90f7271 100644
--- a/webrtc/modules/desktop_capture/screen_drawer_mac.cc
+++ b/webrtc/modules/desktop_capture/screen_drawer_mac.cc
@@ -11,12 +11,14 @@
// TODO(zijiehe): Implement ScreenDrawerMac
#include "webrtc/modules/desktop_capture/screen_drawer.h"
+#include "webrtc/modules/desktop_capture/screen_drawer_lock_posix.h"
+#include "webrtc/rtc_base/ptr_util.h"
namespace webrtc {
// static
std::unique_ptr<ScreenDrawerLock> ScreenDrawerLock::Create() {
- return nullptr;
+ return rtc::MakeUnique<ScreenDrawerLockPosix>();
}
// static
diff --git a/webrtc/modules/desktop_capture/screen_drawer_unittest.cc b/webrtc/modules/desktop_capture/screen_drawer_unittest.cc
index a99395b..94404ee 100644
--- a/webrtc/modules/desktop_capture/screen_drawer_unittest.cc
+++ b/webrtc/modules/desktop_capture/screen_drawer_unittest.cc
@@ -10,16 +10,98 @@
#include "webrtc/modules/desktop_capture/screen_drawer.h"
+#include <atomic>
#include <stdint.h>
+#include "webrtc/rtc_base/checks.h"
+#include "webrtc/rtc_base/function_view.h"
#include "webrtc/rtc_base/logging.h"
#include "webrtc/rtc_base/random.h"
+#include "webrtc/rtc_base/platform_thread.h"
+#include "webrtc/rtc_base/ptr_util.h"
#include "webrtc/rtc_base/timeutils.h"
#include "webrtc/system_wrappers/include/sleep.h"
#include "webrtc/test/gtest.h"
+#if defined(WEBRTC_POSIX)
+#include "webrtc/modules/desktop_capture/screen_drawer_lock_posix.h"
+#endif
+
namespace webrtc {
+namespace {
+
+void TestScreenDrawerLock(
+ rtc::FunctionView<std::unique_ptr<ScreenDrawerLock>()> ctor) {
+ constexpr int kLockDurationMs = 100;
+
+ RTC_DCHECK(ctor);
+
+ std::atomic<bool> created(false);
+ std::atomic<bool> ready(false);
+
+ class Task {
+ public:
+ Task(std::atomic<bool>* created,
+ const std::atomic<bool>& ready,
+ rtc::FunctionView<std::unique_ptr<ScreenDrawerLock>()> ctor)
+ : created_(created),
+ ready_(ready),
+ ctor_(ctor) {}
+
+ ~Task() = default;
+
+ static void RunTask(void* me) {
+ Task* task = static_cast<Task*>(me);
+ std::unique_ptr<ScreenDrawerLock> lock = task->ctor_();
+ ASSERT_TRUE(!!lock);
+ task->created_->store(true);
+ // Wait for the main thread to get the signal of created_.
+ while (!task->ready_.load()) {
+ SleepMs(1);
+ }
+ // At this point, main thread should begin to create a second lock. Though
+ // it's still possible the second lock won't be created before the
+ // following sleep has been finished, the possibility will be
+ // significantly reduced.
+ const int64_t current_ms = rtc::TimeMillis();
+ // SleepMs() may return early. See
+ // https://cs.chromium.org/chromium/src/third_party/webrtc/system_wrappers/include/sleep.h?rcl=4a604c80cecce18aff6fc5e16296d04675312d83&l=20
+ // But we need to ensure at least 100 ms has been passed before unlocking
+ // |lock|.
+ while (rtc::TimeMillis() - current_ms < kLockDurationMs) {
+ SleepMs(kLockDurationMs - (rtc::TimeMillis() - current_ms));
+ }
+ }
+
+ private:
+ std::atomic<bool>* const created_;
+ const std::atomic<bool>& ready_;
+ const rtc::FunctionView<std::unique_ptr<ScreenDrawerLock>()> ctor_;
+ } task(&created, ready, ctor);
+
+ rtc::PlatformThread lock_thread(&Task::RunTask, &task, "lock_thread");
+ lock_thread.Start();
+
+ // Wait for the first lock in Task::RunTask() to be created.
+ // TODO(zijiehe): Find a better solution to wait for the creation of the first
+ // lock. See https://chromium-review.googlesource.com/c/607688/13/webrtc/modules/desktop_capture/screen_drawer_unittest.cc
+ while (!created.load()) {
+ SleepMs(1);
+ }
+
+ const int64_t start_ms = rtc::TimeMillis();
+ ready.store(true);
+ // This is unlikely to fail, but just in case current thread is too laggy and
+ // cause the SleepMs() in RunTask() to finish before we creating another lock.
+ ASSERT_GT(kLockDurationMs, rtc::TimeMillis() - start_ms);
+ ctor();
+ ASSERT_LE(kLockDurationMs, rtc::TimeMillis() - start_ms);
+ lock_thread.Stop();
+}
+
+} // namespace
+
// These are a set of manual test cases, as we do not have an automatical way to
// detect whether a ScreenDrawer on a certain platform works well without
// ScreenCapturer(s). So you may execute these test cases with
@@ -57,4 +139,21 @@
SleepMs(10000);
}
+TEST(ScreenDrawerTest, TwoScreenDrawerLocks) {
+#if defined(WEBRTC_POSIX)
+ // ScreenDrawerLockPosix won't be able to unlink the named semaphore. So use a
+ // different semaphore name here to avoid deadlock.
+ const char* semaphore_name = "GSDL8784541a812011e788ff67427b";
+ ScreenDrawerLockPosix::Unlink(semaphore_name);
+
+ TestScreenDrawerLock([semaphore_name]() {
+ return rtc::MakeUnique<ScreenDrawerLockPosix>(semaphore_name);
+ });
+#elif defined(WEBRTC_WIN)
+ TestScreenDrawerLock([]() {
+ return ScreenDrawerLock::Create();
+ });
+#endif
+}
+
} // namespace webrtc
