From 8701ed0270dd8da44efff5e19f3b0b6d3cac5b8b Mon Sep 17 00:00:00 2001
From: James Zern <jzern@google.com>
Date: Wed, 9 Jul 2014 12:45:21 -0700
Subject: [PATCH] update vp9_thread.c

pull the latest from libwebp.

Original source:
 http://git.chromium.org/webm/libwebp.git
 100644 blob 264210ba2807e4da47eb5d18c04cf869d89b9784 src/utils/thread.c

commit 46fd44c1042c9903b2f1ab87e9f200a13c7e702d
Author: James Zern <jzern@google.com>
Date:   Tue Jul 8 19:53:28 2014 -0700

    thread: remove harmless race on status_ in End()

    if a thread was still doing work when End() was called there'd be a race
    on worker->status_. in these cases, however, the specific value is
    meaningless as it would be >= OK and the thread would have been shut
    down properly, but we'll check 'impl_' instead to avoid any potential
    TSan/DRD reports.

    Change-Id: Ib93cbc226a099f07761f7bad765549dffb8054b1

Change-Id: Ib0ef25737b3c6d017fa74822e21ed58508230b91
---
 test/vp9_thread_test.cc | 56 ++++++++++++++++++++++++++++++++---------
 vp9/common/vp9_thread.c | 13 +++++-----
 2 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/test/vp9_thread_test.cc b/test/vp9_thread_test.cc
index fa51835a5..d7fc4eedb 100644
--- a/test/vp9_thread_test.cc
+++ b/test/vp9_thread_test.cc
@@ -35,6 +35,15 @@ class VP9WorkerThreadTest : public ::testing::TestWithParam<bool> {
     vp9_get_worker_interface()->end(&worker_);
   }
 
+  void Run(VP9Worker* worker) {
+    const bool synchronous = GetParam();
+    if (synchronous) {
+      vp9_get_worker_interface()->execute(worker);
+    } else {
+      vp9_get_worker_interface()->launch(worker);
+    }
+  }
+
   VP9Worker worker_;
 };
 
@@ -57,12 +66,7 @@ TEST_P(VP9WorkerThreadTest, HookSuccess) {
     worker_.data1 = &hook_data;
     worker_.data2 = &return_value;
 
-    const bool synchronous = GetParam();
-    if (synchronous) {
-      vp9_get_worker_interface()->execute(&worker_);
-    } else {
-      vp9_get_worker_interface()->launch(&worker_);
-    }
+    Run(&worker_);
     EXPECT_NE(vp9_get_worker_interface()->sync(&worker_), 0);
     EXPECT_FALSE(worker_.had_error);
     EXPECT_EQ(5, hook_data);
@@ -81,12 +85,7 @@ TEST_P(VP9WorkerThreadTest, HookFailure) {
   worker_.data1 = &hook_data;
   worker_.data2 = &return_value;
 
-  const bool synchronous = GetParam();
-  if (synchronous) {
-    vp9_get_worker_interface()->execute(&worker_);
-  } else {
-    vp9_get_worker_interface()->launch(&worker_);
-  }
+  Run(&worker_);
   EXPECT_FALSE(vp9_get_worker_interface()->sync(&worker_));
   EXPECT_EQ(1, worker_.had_error);
 
@@ -99,6 +98,39 @@ TEST_P(VP9WorkerThreadTest, HookFailure) {
   EXPECT_FALSE(worker_.had_error);
 }
 
+TEST_P(VP9WorkerThreadTest, EndWithoutSync) {
+  // Create a large number of threads to increase the chances of detecting a
+  // race. Doing more work in the hook is no guarantee as any race would occur
+  // post hook execution in the main thread loop driver.
+  static const int kNumWorkers = 64;
+  VP9Worker workers[kNumWorkers];
+  int hook_data[kNumWorkers];
+  int return_value[kNumWorkers];
+
+  for (int n = 0; n < kNumWorkers; ++n) {
+    vp9_get_worker_interface()->init(&workers[n]);
+    return_value[n] = 1;  // return successfully from the hook
+    workers[n].hook = ThreadHook;
+    workers[n].data1 = &hook_data[n];
+    workers[n].data2 = &return_value[n];
+  }
+
+  for (int i = 0; i < 2; ++i) {
+    for (int n = 0; n < kNumWorkers; ++n) {
+      EXPECT_NE(vp9_get_worker_interface()->reset(&workers[n]), 0);
+      hook_data[n] = 0;
+    }
+
+    for (int n = 0; n < kNumWorkers; ++n) {
+      Run(&workers[n]);
+    }
+
+    for (int n = kNumWorkers - 1; n >= 0; --n) {
+      vp9_get_worker_interface()->end(&workers[n]);
+    }
+  }
+}
+
 TEST(VP9WorkerThreadTest, TestInterfaceAPI) {
   EXPECT_EQ(0, vp9_set_worker_interface(NULL));
   EXPECT_TRUE(vp9_get_worker_interface() != NULL);
diff --git a/vp9/common/vp9_thread.c b/vp9/common/vp9_thread.c
index 348bdf6db..1c6aec032 100644
--- a/vp9/common/vp9_thread.c
+++ b/vp9/common/vp9_thread.c
@@ -11,7 +11,7 @@
 //
 // Original source:
 //  http://git.chromium.org/webm/libwebp.git
-//  100644 blob 08ad4e1fecba302bf1247645e84a7d2779956bc3  src/utils/thread.c
+//  100644 blob 264210ba2807e4da47eb5d18c04cf869d89b9784  src/utils/thread.c
 
 #include <assert.h>
 #include <string.h>   // for memset()
@@ -144,18 +144,19 @@ static void launch(VP9Worker *const worker) {
 }
 
 static void end(VP9Worker *const worker) {
-  if (worker->status_ >= OK) {
 #if CONFIG_MULTITHREAD
+  if (worker->impl_ != NULL) {
     change_state(worker, NOT_OK);
     pthread_join(worker->impl_->thread_, NULL);
     pthread_mutex_destroy(&worker->impl_->mutex_);
     pthread_cond_destroy(&worker->impl_->condition_);
+    vpx_free(worker->impl_);
+    worker->impl_ = NULL;
+  }
 #else
-    worker->status_ = NOT_OK;
+  worker->status_ = NOT_OK;
+  assert(worker->impl_ == NULL);
 #endif
-  }
-  vpx_free(worker->impl_);
-  worker->impl_ = NULL;
   assert(worker->status_ == NOT_OK);
 }
 
-- 
2.40.0