Cleanup unit tests, and remove the loop that checks to make sure all jobs are run.

PiperOrigin-RevId: 397869801

cpp/platform/impl/windows/BUILD

@@ -150,6 +150,7 @@ cc_library(
         "test_utils.cc",
     ],
     hdrs = [
+        "test_data.h",
         "test_utils.h",
     ],
     visibility = [

cpp/platform/impl/windows/executor_test.cc

@@ -1,4 +1,4 @@
-// Copyright 2020 Google LLC
+// Copyright 2021 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,11 +17,13 @@
 #include <algorithm>
 #include <utility>
 
+#include "platform/impl/windows/test_data.h"
+
 #include "gtest/gtest.h"
 
 TEST(ExecutorTests, SingleThreadedExecutorSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::Executor> executor =
       std::make_unique<location::nearby::windows::Executor>();
@@ -35,7 +37,7 @@ TEST(ExecutorTests, SingleThreadedExecutorSucceeds) {
   // Act
   executor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
 
   executor->Shutdown();
@@ -67,7 +69,7 @@ TEST(ExecutorTests, SingleThreadedExecutorAfterShutdownFails) {
   // Act
   executor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1");
+    output->append(RUNNABLE_0_TEXT.c_str());
   });
 
   //  Assert
@@ -82,7 +84,7 @@ TEST(ExecutorTests, SingleThreadedExecutorAfterShutdownFails) {
 
 TEST(ExecutorTests, SingleThreadedExecutorExecuteNullSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::Executor> executor =
       std::make_unique<location::nearby::windows::Executor>();
@@ -97,7 +99,7 @@ TEST(ExecutorTests, SingleThreadedExecutorExecuteNullSucceeds) {
   executor->Execute(nullptr);
   executor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
   executor->Execute(nullptr);
 
@@ -115,8 +117,7 @@ TEST(ExecutorTests, SingleThreadedExecutorExecuteNullSucceeds) {
 
 TEST(ExecutorTests, SingleThreadedExecutorMultipleTasksSucceeds) {
   // Arrange
-  std::string expected(
-      "runnable 1, runnable 2, runnable 3, runnable 4, runnable 5");
+  std::string expected(RUNNABLE_ALL_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::Executor> executor =
       std::make_unique<location::nearby::windows::Executor>();
@@ -128,26 +129,14 @@ TEST(ExecutorTests, SingleThreadedExecutorMultipleTasksSucceeds) {
   threadIds->push_back(GetCurrentThreadId());
 
   // Act
-  executor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1, ");
-  });
-  executor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 2, ");
-  });
-  executor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 3, ");
-  });
-  executor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 4, ");
-  });
-  executor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 5");
-  });
+  for (int index = 0; index < 5; index++) {
+    executor->Execute([&output, &threadIds, index]() {
+      threadIds->push_back(GetCurrentThreadId());
+      char buffer[128];
+      snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index);
+      output.append(std::string(buffer));
+    });
+  }
 
   executor->Shutdown();
 
@@ -169,7 +158,7 @@ TEST(ExecutorTests, SingleThreadedExecutorMultipleTasksSucceeds) {
 
 TEST(ExecutorTests, MultiThreadedExecutorSingleTaskSucceeds) {
   //  Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::Executor> executor =
       std::make_unique<location::nearby::windows::Executor>(2);
@@ -185,7 +174,7 @@ TEST(ExecutorTests, MultiThreadedExecutorSingleTaskSucceeds) {
   //  Act
   executor->Execute([output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1");
+    output->append(RUNNABLE_0_TEXT.c_str());
   });
 
   executor->Shutdown();
@@ -214,26 +203,14 @@ TEST(ExecutorTests, MultiThreadedExecutorMultipleTasksSucceeds) {
   threadIds->push_back(GetCurrentThreadId());
 
   //  Act
-  executor->Execute([output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1, ");
-  });
-  executor->Execute([output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 2, ");
-  });
-  executor->Execute([output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 3, ");
-  });
-  executor->Execute([output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 4, ");
-  });
-  executor->Execute([output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 5");
-  });
+  for (int index = 0; index < 5; index++) {
+    executor->Execute([&output, &threadIds, index]() {
+      threadIds->push_back(GetCurrentThreadId());
+      char buffer[128];
+      snprintf(buffer, sizeof(buffer), "%s %d, ", RUNNABLE_TEXT.c_str(), index);
+      output->append(std::string(buffer));
+    });
+  }
 
   executor->Shutdown();
 
@@ -265,7 +242,7 @@ TEST(ExecutorTests, MultiThreadedExecutorSingleTaskAfterShutdownFails) {
   //  Act
   executor->Execute([output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1");
+    output->append(RUNNABLE_0_TEXT.c_str());
   });
 
   //  Assert
@@ -289,7 +266,7 @@ TEST(ExecutorTests, MultiThreadedExecutorNegativeThreadsThrows) {
               std::make_unique<location::nearby::windows::Executor>(-1);
         } catch (const std::invalid_argument::exception& e) {
           // and this tests that it has the correct message
-          EXPECT_STREQ("max_concurrency", e.what());
+          EXPECT_STREQ(INVALID_ARGUMENT_TEXT, e.what());
           throw;
         }
       },
@@ -307,7 +284,7 @@ TEST(ExecutorTests, MultiThreadedExecutorTooManyThreadsThrows) {
               std::make_unique<location::nearby::windows::Executor>(65);
         } catch (const location::nearby::windows::ThreadPoolException& e) {
           // and this tests that it has the correct message
-          EXPECT_STREQ("Thread pool max size exceeded.", e.what());
+          EXPECT_STREQ(THREADPOOL_MAX_SIZE_TEXT, e.what());
           throw;
         }
       },
@@ -340,9 +317,9 @@ TEST(ExecutorTests,
           EnterCriticalSection(&testCriticalSection);
 
           threadIds.push_back(id);
-          output->append("runnable ");
+          output->append(RUNNABLE_TEXT);
           output->append(std::to_string(index));
-          output->append(", ");
+          output->append(RUNNABLE_SEPARATOR_TEXT);
 
           LeaveCriticalSection(&testCriticalSection);
           // Using rand since this is in a critical section

cpp/platform/impl/windows/scheduled_executor.cc

@@ -58,7 +58,7 @@ void WINAPI ScheduledExecutor::_TimerProc(LPVOID argToCompletionRoutine,
   _ASSERT(argToCompletionRoutine != NULL);
   if (NULL == argToCompletionRoutine) {
     NEARBY_LOGS(ERROR)
-        << "Error: " << __func__
+        << __func__
         << ": TimerProc argument argToCompletionRoutine was null.";
 
     return;
@@ -84,6 +84,10 @@ ScheduledExecutor::ScheduledExecutor()
 std::shared_ptr<api::Cancelable> ScheduledExecutor::Schedule(
     Runnable&& runnable, absl::Duration duration) {
   if (shut_down_) {
+    NEARBY_LOGS(ERROR)
+        << __func__
+        << ": Attempt to Schedule on a shut down executor.";
+
     return nullptr;
   }
 
@@ -122,6 +126,8 @@ std::shared_ptr<api::Cancelable> ScheduledExecutor::Schedule(
 // https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Executor.html#execute-java.lang.Runnable-
 void ScheduledExecutor::Execute(Runnable&& runnable) {
   if (shut_down_) {
+    NEARBY_LOGS(ERROR) << __func__
+                       << ": Attempt to Execute on a shut down executor.";
     return;
   }
 
@@ -133,7 +139,10 @@ void ScheduledExecutor::Shutdown() {
   if (!shut_down_) {
     shut_down_ = true;
     executor_->Shutdown();
+    return;
   }
+  NEARBY_LOGS(ERROR) << __func__
+                     << ": Attempt to Shutdown on a shut down executor.";
 }
 }  // namespace windows
 }  // namespace nearby

cpp/platform/impl/windows/scheduled_executor_test.cc

@@ -15,11 +15,13 @@
 
 #include <utility>
 
+#include "platform/impl/windows/test_data.h"
+
 #include "gtest/gtest.h"
 
 TEST(ScheduledExecutorTests, ExecuteSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::ScheduledExecutor>
       submittableExecutor =
@@ -34,7 +36,7 @@ TEST(ScheduledExecutorTests, ExecuteSucceeds) {
   // Act
   submittableExecutor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
 
   submittableExecutor->Shutdown();
@@ -51,7 +53,7 @@ TEST(ScheduledExecutorTests, ExecuteSucceeds) {
 
 TEST(ScheduledExecutorTests, ScheduleSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::ScheduledExecutor>
       submittableExecutor =
@@ -72,7 +74,7 @@ TEST(ScheduledExecutorTests, ScheduleSucceeds) {
       [&output, &threadIds, &timeExecuted]() {
         timeExecuted = std::chrono::system_clock::now();
         threadIds->push_back(GetCurrentThreadId());
-        output.append("runnable 1");
+        output.append(RUNNABLE_0_TEXT.c_str());
       },
       absl::Milliseconds(50));
 
@@ -115,7 +117,7 @@ TEST(ScheduledExecutorTests, CancelSucceeds) {
   auto cancelable = submittableExecutor->Schedule(
       [&output, &threadIds]() {
         threadIds->push_back(GetCurrentThreadId());
-        output.append("runnable 1");
+        output.append(RUNNABLE_0_TEXT.c_str());
       },
       absl::Milliseconds(1000));
 
@@ -136,7 +138,7 @@ TEST(ScheduledExecutorTests, CancelSucceeds) {
 
 TEST(ScheduledExecutorTests, CancelAfterStartedFails) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::ScheduledExecutor>
       submittableExecutor =
@@ -152,7 +154,7 @@ TEST(ScheduledExecutorTests, CancelAfterStartedFails) {
   auto cancelable = submittableExecutor->Schedule(
       [&output, &threadIds]() {
         threadIds->push_back(GetCurrentThreadId());
-        output.append("runnable 1");
+        output.append(RUNNABLE_0_TEXT.c_str());
       },
       absl::Milliseconds(100));
 

cpp/platform/impl/windows/submittable_executor.cc

@@ -33,6 +33,9 @@ bool SubmittableExecutor::DoSubmit(Runnable&& wrapped_callable) {
     return true;
   }
 
+  NEARBY_LOGS(ERROR) << "Error: " << __func__
+                     << ": Attempt to DoSubmit on a shutdown executor.";
+
   return false;
 }
 
@@ -52,6 +55,9 @@ void SubmittableExecutor::Shutdown() {
     executor_->Shutdown();
     shut_down_ = true;
   }
+
+  NEARBY_LOGS(ERROR) << "Error: " << __func__
+                     << ": Attempt to Shutdown on a shutdown executor.";
 }
 
 }  // namespace windows

cpp/platform/impl/windows/submittable_executor_test.cc

@@ -15,11 +15,13 @@
 
 #include <utility>
 
+#include "platform/impl/windows/test_data.h"
+
 #include "gtest/gtest.h"
 
 TEST(SubmittableExecutorTests, SingleThreadedExecuteSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::SubmittableExecutor>
       submittableExecutor =
@@ -34,7 +36,7 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteSucceeds) {
   // Act
   submittableExecutor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
 
   submittableExecutor->Shutdown();
@@ -68,11 +70,9 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteAfterShutdownFails) {
   // Act
   submittableExecutor->Execute([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
 
-  Sleep(1);  //  Yield the thread
-
   //  Assert
   //  We should've run 1 time on the main thread, and 0 times on the
   //  workerThread
@@ -85,7 +85,7 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteAfterShutdownFails) {
 
 TEST(SubmittableExecutorTests, SingleThreadedDoSubmitSucceeds) {
   // Arrange
-  std::string expected("runnable 1");
+  std::string expected(RUNNABLE_0_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::SubmittableExecutor>
       submittableExecutor =
@@ -100,7 +100,7 @@ TEST(SubmittableExecutorTests, SingleThreadedDoSubmitSucceeds) {
   // Act
   auto result = submittableExecutor->DoSubmit([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output.append("runnable 1");
+    output.append(RUNNABLE_0_TEXT.c_str());
   });
 
   submittableExecutor->Shutdown();
@@ -137,7 +137,7 @@ TEST(SubmittableExecutorTests,
   // Act
   auto result = submittableExecutor->DoSubmit([&output, &threadIds]() {
     threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1");
+    output->append(RUNNABLE_0_TEXT.c_str());
   });
 
   //  Assert
@@ -154,8 +154,7 @@ TEST(SubmittableExecutorTests,
 
 TEST(SubmittableExecutorTests, SingleThreadedExecuteMultipleTasksSucceeds) {
   // Arrange
-  std::string expected(
-      "runnable 1, runnable 2, runnable 3, runnable 4, runnable 5");
+  std::string expected(RUNNABLE_ALL_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::SubmittableExecutor>
       submittableExecutor =
@@ -168,26 +167,14 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteMultipleTasksSucceeds) {
   threadIds->push_back(GetCurrentThreadId());
 
   // Act
-  submittableExecutor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1, ");
-  });
-  submittableExecutor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 2, ");
-  });
-  submittableExecutor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 3, ");
-  });
-  submittableExecutor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 4, ");
-  });
-  submittableExecutor->Execute([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 5");
-  });
+  for (int index = 0; index < 5; index++) {
+    submittableExecutor->Execute([&output, &threadIds, index]() {
+      threadIds->push_back(GetCurrentThreadId());
+      char buffer[128];
+      snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index);
+      output->append(std::string(buffer));
+    });
+  }
 
   submittableExecutor->Shutdown();
 
@@ -209,8 +196,7 @@ TEST(SubmittableExecutorTests, SingleThreadedExecuteMultipleTasksSucceeds) {
 
 TEST(SubmittableExecutorTests, SingleThreadedDoSubmitMultipleTasksSucceeds) {
   // Arrange
-  std::string expected(
-      "runnable 1, runnable 2, runnable 3, runnable 4, runnable 5");
+  std::string expected(RUNNABLE_ALL_TEXT.c_str());
 
   std::unique_ptr<location::nearby::windows::SubmittableExecutor>
       submittableExecutor =
@@ -223,26 +209,15 @@ TEST(SubmittableExecutorTests, SingleThreadedDoSubmitMultipleTasksSucceeds) {
   threadIds->push_back(GetCurrentThreadId());
 
   // Act
-  auto result = submittableExecutor->DoSubmit([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 1, ");
-  });
-  result |= submittableExecutor->DoSubmit([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 2, ");
-  });
-  result |= submittableExecutor->DoSubmit([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 3, ");
-  });
-  result |= submittableExecutor->DoSubmit([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 4, ");
-  });
-  result |= submittableExecutor->DoSubmit([&output, &threadIds]() {
-    threadIds->push_back(GetCurrentThreadId());
-    output->append("runnable 5");
-  });
+  bool result = true;
+  for (int index = 0; index < 5; index++) {
+    result &= submittableExecutor->DoSubmit([&output, &threadIds, index]() {
+      threadIds->push_back(GetCurrentThreadId());
+      char buffer[128];
+      snprintf(buffer, sizeof(buffer), "%s%d, ", RUNNABLE_TEXT.c_str(), index);
+      output->append(std::string(buffer));
+    });
+  }
 
   submittableExecutor->Shutdown();
 

cpp/platform/impl/windows/test_data.h

@@ -0,0 +1,34 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef PLATFORM_IMPL_WINDOWS_TEST_DATA_H_
+#define PLATFORM_IMPL_WINDOWS_TEST_DATA_H_
+
+#define INVALID_ARGUMENT_TEXT "max_concurrency"
+#define THREADPOOL_MAX_SIZE_TEXT "Thread pool max size exceeded."
+#define RUNNABLE_TEXT std::string("runnable ")
+#define RUNNABLE_0_TEXT RUNNABLE_TEXT + std::string("0")
+#define RUNNABLE_1_TEXT RUNNABLE_TEXT + std::string("1")
+#define RUNNABLE_2_TEXT RUNNABLE_TEXT + std::string("2")
+#define RUNNABLE_3_TEXT RUNNABLE_TEXT + std::string("3")
+#define RUNNABLE_4_TEXT RUNNABLE_TEXT + std::string("4")
+#define RUNNABLE_SEPARATOR_TEXT std::string(", ")
+#define RUNNABLE_ALL_TEXT                                                \
+  (RUNNABLE_0_TEXT + RUNNABLE_SEPARATOR_TEXT + RUNNABLE_1_TEXT +         \
+   RUNNABLE_SEPARATOR_TEXT + RUNNABLE_2_TEXT + RUNNABLE_SEPARATOR_TEXT + \
+   RUNNABLE_3_TEXT + RUNNABLE_SEPARATOR_TEXT + RUNNABLE_4_TEXT +         \
+   RUNNABLE_SEPARATOR_TEXT)
+
+#endif  // PLATFORM_IMPL_WINDOWS_TEST_DATA_H_
+

cpp/platform/impl/windows/thread_pool.cc

@@ -1,4 +1,4 @@
-// Copyright 2020 Google LLC
+// Copyright 2021 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -41,6 +41,7 @@ DWORD WINAPI ThreadPool::_ThreadProc(LPVOID pParam) {
 
   _ASSERT(pParam != NULL);
   if (NULL == pParam) {
+    NEARBY_LOGS(ERROR) << __func__ << ": pParam must not be null.";
     return -1;
   }
 
@@ -94,6 +95,7 @@ ThreadPool::ThreadPool(int nPoolSize, bool bCreateNow)
   // windows has a max of 64. This means we can only wait on up
   // to 64 threads, anything more gives undesirable results.
   if (nPoolSize > 63) {
+    NEARBY_LOGS(ERROR) << __func__ << ": Thread pool max size exceeded.";
     throw ThreadPoolException("Thread pool max size exceeded.");
   }
 
@@ -104,14 +106,18 @@ ThreadPool::ThreadPool(int nPoolSize, bool bCreateNow)
 
   if (bCreateNow) {
     if (!Create()) {
-      throw ThreadPoolException("Thread pool creation failed");
+      NEARBY_LOGS(ERROR) << __func__ << ": Thread pool creation failed.";
+      throw ThreadPoolException("Thread pool creation failed.");
     }
   }
 }
 
 bool ThreadPool::Create() {
   if (pool_state_ != State::Destroyed) {
-    // To create a new pool, destory the existing one first
+    // To create a new pool, destroy the existing one first
+    NEARBY_LOGS(ERROR) << __func__
+                       << ": Attempt to create a new thread pool before "
+                          "destroying the old one.";
     return false;
   }
 
@@ -241,13 +247,6 @@ void ThreadPool::Destroy() {
 
   bool notDone = true;
 
-  while (notDone) {
-    EnterCriticalSection(&critical_section_);
-    notDone = function_list_->size() > 0;
-    LeaveCriticalSection(&critical_section_);
-    Sleep(100);
-  }
-
   EnterCriticalSection(&critical_section_);
 
   ThreadMap::iterator iter = thread_map_->begin();
@@ -335,6 +334,8 @@ void ThreadPool::SetPoolSize(int nSize) {
   _ASSERT(nSize > 0);
 
   if (nSize <= 0) {
+    NEARBY_LOGS(ERROR)
+        << __func__ << ": 0 or negative value is not a valid thread pool size.";
     return;
   }
 
@@ -382,6 +383,7 @@ void ThreadPool::FinishNotify(DWORD threadId) {
   if (threadMapIterator == thread_map_->end())  // if search found no elements
   {
     _ASSERT(!"No matching thread found.");
+    NEARBY_LOGS(ERROR) << __func__ << ": No matching thread found.";
   } else {
     thread_map_->at(threadId)->free = true;