From d2bc79c03a0b09c259d84fe4ea83b62e74ca9eca Mon Sep 17 00:00:00 2001
From: Andrew Krieger <akrieger@fb.com>
Date: Wed, 22 Jan 2020 16:43:14 -0800
Subject: [PATCH] Use extended alignment safe allocator in
 CPUThreadPoolExecutor

Summary:
Until C++17 types with extended alignment need special handling
in eg. std::vector and also std::unique_ptr. CPUThreadPoolExecutor has a
default queue that requires extended alignment, but also allows the user
to provide their own blocking queue. To handle this we move the private
member to a shared_ptr which supported type erased destructors, and then
use allocate_shared for the default queue type with a custom allocator
that will satisfy alignment constraints.

```
.../unique_ptr.h:825:34: runtime error: constructor call on misaligned address 0x613000000040 for type 'folly::UnboundedBlockingQueue<folly::CPUThreadPoolExecutor::CPUTask>', which requires 128 byte alignment
0x613000000040: note: pointer points here
 02 00 00 1c  be be be be be be be be  be be be be be be be be  be be be be be be be be  be be be be
              ^
    #0 0x75b35e in std::_MakeUniq<folly::UnboundedBlockingQueue<folly::CPUThreadPoolExecutor::CPUTask> >::__single_object std::make_unique<folly::UnboundedBlockingQueue<folly::CPUThreadPoolExecutor::CPUTask> >() .../unique_ptr.h:825:30
    #1 0x75602f in folly::CPUThreadPoolExecutor::CPUThreadPoolExecutor(unsigned long, std::shared_ptr<folly::ThreadFactory>) xplat/folly/executors/CPUThreadPoolExecutor.cpp:66:18
    #2 0x756c6c in folly::CPUThreadPoolExecutor::CPUThreadPoolExecutor(unsigned long) xplat/folly/executors/CPUThreadPoolExecutor.cpp:84:7
```

Differential Revision: D19237477

fbshipit-source-id: c88b26e2ca17e168f124ba27c0989f787b1ce4fd
---
 folly/executors/CPUThreadPoolExecutor.cpp | 19 +++++++++++++++----
 folly/executors/CPUThreadPoolExecutor.h   |  3 ++-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/folly/executors/CPUThreadPoolExecutor.cpp b/folly/executors/CPUThreadPoolExecutor.cpp
index 7d7fb90deec..1b2403ce760 100644
--- a/folly/executors/CPUThreadPoolExecutor.cpp
+++ b/folly/executors/CPUThreadPoolExecutor.cpp
@@ -16,6 +16,7 @@
 
 #include <folly/executors/CPUThreadPoolExecutor.h>
 
+#include <folly/Memory.h>
 #include <folly/executors/task_queue/PriorityLifoSemMPMCQueue.h>
 #include <folly/executors/task_queue/PriorityUnboundedBlockingQueue.h>
 #include <folly/executors/task_queue/UnboundedBlockingQueue.h>
@@ -28,6 +29,16 @@ DEFINE_bool(
 
 namespace folly {
 
+namespace {
+//  queue_alloc custom allocator is necessary until C++17
+//    http://open-std.org/JTC1/SC22/WG21/docs/papers/2012/n3396.htm
+//    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65122
+//    https://bugs.llvm.org/show_bug.cgi?id=22634
+using default_queue = UnboundedBlockingQueue<CPUThreadPoolExecutor::CPUTask>;
+using default_queue_alloc =
+    AlignedSysAllocator<default_queue, FixedAlign<alignof(default_queue)>>;
+} // namespace
+
 const size_t CPUThreadPoolExecutor::kDefaultMaxQueueSize = 1 << 14;
 
 CPUThreadPoolExecutor::CPUThreadPoolExecutor(
@@ -38,7 +49,7 @@ CPUThreadPoolExecutor::CPUThreadPoolExecutor(
           numThreads,
           FLAGS_dynamic_cputhreadpoolexecutor ? 0 : numThreads,
           std::move(threadFactory)),
-      taskQueue_(std::move(taskQueue)) {
+      taskQueue_(taskQueue.release()) {
   setNumThreads(numThreads);
   registerThreadPoolExecutor(this);
 }
@@ -51,7 +62,7 @@ CPUThreadPoolExecutor::CPUThreadPoolExecutor(
           numThreads.first,
           numThreads.second,
           std::move(threadFactory)),
-      taskQueue_(std::move(taskQueue)) {
+      taskQueue_(taskQueue.release()) {
   setNumThreads(numThreads.first);
   registerThreadPoolExecutor(this);
 }
@@ -63,7 +74,7 @@ CPUThreadPoolExecutor::CPUThreadPoolExecutor(
           numThreads,
           FLAGS_dynamic_cputhreadpoolexecutor ? 0 : numThreads,
           std::move(threadFactory)),
-      taskQueue_(std::make_unique<UnboundedBlockingQueue<CPUTask>>()) {
+      taskQueue_(std::allocate_shared<default_queue>(default_queue_alloc{})) {
   setNumThreads(numThreads);
   registerThreadPoolExecutor(this);
 }
@@ -75,7 +86,7 @@ CPUThreadPoolExecutor::CPUThreadPoolExecutor(
           numThreads.first,
           numThreads.second,
           std::move(threadFactory)),
-      taskQueue_(std::make_unique<UnboundedBlockingQueue<CPUTask>>()) {
+      taskQueue_(std::allocate_shared<default_queue>(default_queue_alloc{})) {
   setNumThreads(numThreads.first);
   registerThreadPoolExecutor(this);
 }
diff --git a/folly/executors/CPUThreadPoolExecutor.h b/folly/executors/CPUThreadPoolExecutor.h
index cbdfb61734a..b3a6c2b704a 100644
--- a/folly/executors/CPUThreadPoolExecutor.h
+++ b/folly/executors/CPUThreadPoolExecutor.h
@@ -146,7 +146,8 @@ class CPUThreadPoolExecutor : public ThreadPoolExecutor {
   bool tryDecrToStop();
   bool taskShouldStop(folly::Optional<CPUTask>&);
 
-  std::unique_ptr<BlockingQueue<CPUTask>> taskQueue_;
+  // shared_ptr for type erased dtor to handle extended alignment.
+  std::shared_ptr<BlockingQueue<CPUTask>> taskQueue_;
   std::atomic<ssize_t> threadsToStop_{0};
 };