From cb6389d42d0b1652f8d2ca56b461fc99e9e43a42 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <anoyes34@gmail.com>
Date: Fri, 17 Apr 2020 23:34:28 +0000
Subject: [PATCH] Prevent main thread from destroying flatbuffers globals

We recently witnessed (using tsan) the main thread exiting without first
joining the network thread, and this caused data races and
heap-use-after-free's

Now the lifetime of these globals will be tied to the network thread
itself (and I guess every thread, but the one that actually uses memory
will be owned by the network thread.)
---
 flow/flat_buffers.cpp |  6 ++++--
 flow/flat_buffers.h   | 11 ++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/flow/flat_buffers.cpp b/flow/flat_buffers.cpp
index 1cb4b1099d..8871fa438e 100644
--- a/flow/flat_buffers.cpp
+++ b/flow/flat_buffers.cpp
@@ -31,10 +31,12 @@
 namespace detail {
 
 namespace {
-std::vector<int> mWriteToOffsetsMemoy;
+thread_local std::vector<int> gWriteToOffsetsMemory;
 }
 
-std::vector<int>* writeToOffsetsMemory = &mWriteToOffsetsMemoy;
+void swapWithThreadLocalGlobal(std::vector<int>& writeToOffsets) {
+	gWriteToOffsetsMemory.swap(writeToOffsets);
+}
 
 VTable generate_vtable(size_t numMembers, const std::vector<unsigned>& sizesAlignments) {
 	if (numMembers == 0) {
diff --git a/flow/flat_buffers.h b/flow/flat_buffers.h
index ff8f7ccceb..193813edd6 100644
--- a/flow/flat_buffers.h
+++ b/flow/flat_buffers.h
@@ -343,15 +343,16 @@ struct _SizeOf {
 	static constexpr unsigned int align = fb_align<T>;
 };
 
-extern std::vector<int>* writeToOffsetsMemory;
+// Re-use this intermediate memory to avoid frequent new/delete
+void swapWithThreadLocalGlobal(std::vector<int>& writeToOffsets);
 
 template <class Context>
 struct PrecomputeSize : Context {
 	PrecomputeSize(const Context& context) : Context(context) {
-		writeToOffsets.swap(*writeToOffsetsMemory);
+		swapWithThreadLocalGlobal(writeToOffsets);
 		writeToOffsets.clear();
 	}
-	~PrecomputeSize() { writeToOffsets.swap(*writeToOffsetsMemory); }
+	~PrecomputeSize() { swapWithThreadLocalGlobal(writeToOffsets); }
 	// |offset| is measured from the end of the buffer. Precondition: len <=
 	// offset.
 	void write(const void*, int offset, int /*len*/) { current_buffer_size = std::max(current_buffer_size, offset); }
@@ -491,7 +492,7 @@ extern VTable generate_vtable(size_t numMembers, const std::vector<unsigned>& si
 
 template <unsigned... MembersAndAlignments>
 const VTable* gen_vtable3() {
-	static VTable table =
+	static thread_local VTable table =
 	    generate_vtable(sizeof...(MembersAndAlignments) / 2, std::vector<unsigned>{ MembersAndAlignments... });
 	return &table;
 }
@@ -619,7 +620,7 @@ VTableSet get_vtableset_impl(const Root& root, const Context& context) {
 
 template <class Root, class Context>
 const VTableSet* get_vtableset(const Root& root, const Context& context) {
-	static VTableSet result = get_vtableset_impl(root, context);
+	static thread_local VTableSet result = get_vtableset_impl(root, context);
 	return &result;
 }