Fix use-after-move issue in AsyncTaskExecutor

`getFuture()` should be called before post as `send`/`sendError`
operation in `ThreadReturnPromise` moves the underlying Promise to
`tagAndForward()`.

Ideally, `ThreadReturnPromise` behavior should stay consistent with the
`Promise`. However, the problem is that it relies on the invariant that
there will always be one owner of its internal `Promise` which is either
itself or `tagOrForward`  -- which is necessary to ensure that only one
thread can operate on the Promise's internal state (ref count, flags
etc) and avoid race conditions.

This patch (1) makes sure that in case of `post()` function we get
future before, (2) adds an ASSERT as this should never happen, (3)
documentation for future users and (4) a test case for potentially
fixing this in future.

fdbrpc/include/fdbrpc/grpc/AsyncTaskExecutor.h

@@ -83,8 +83,9 @@ public:
 	[[nodiscard]] auto post(Func&& task) -> Future<typename std::invoke_result<Func>::type> {
 		ASSERT_WE_THINK(g_network->isOnMainThread());
 		auto action = new Action<Func>(std::forward<Func>(task));
+		auto future = action->getFuture();
 		pool_->post(action);
-		return action->getFuture();
+		return future;
 	}
 
 	// Schedules a function that returns void for asynchronous execution in a thread pool.
@@ -122,7 +123,7 @@ template <typename Func>
 struct AsyncTaskExecutor::Action<Func, typename std::enable_if_t<!IsVoidReturn<Func>>> : ThreadAction {
 	using Ret = std::invoke_result<Func>::type;
 
-	Action(Func&& fn) : fn_(std::forward<Func>(fn)) {}
+	Action(Func&& fn) : fn_(std::move(fn)) {}
 
 	void operator()(IThreadPoolReceiver* action) override {
 		// TODO: Should we abort if there are no future references?
@@ -159,7 +160,7 @@ template <typename Func>
 struct AsyncTaskExecutor::Action<Func, typename std::enable_if_t<IsVoidReturn<Func>>> : ThreadAction {
 	using Ret = std::invoke_result<Func>::type;
 
-	Action(Func&& fn) : fn_(std::forward<Func>(fn)) {}
+	Action(Func&& fn) : fn_(std::move(fn)) {}
 
 	void operator()(IThreadPoolReceiver* action) override {
 		fn_();

flow/IThreadPoolTest.actor.cpp

@@ -229,6 +229,18 @@ TEST_CASE("/flow/IThreadPool/ThreadReturnPromiseStream_DestroyPromise") {
 	return Void();
 }
 
+// FIXME: Someday fix ThreadReturnPromise for this testcase.
+// TEST_CASE("/flow/IThreadPool/ThreadReturnPromiseGetFutureAfterSend") {
+// 	auto get_f = [&]() {
+// 		ThreadReturnPromise<int> p;
+// 		p.send(1234);
+// 		return p.getFuture();
+// 	};
+// 	int res = wait(get_f());
+// 	ASSERT_EQ(res, 1234);
+// 	return Void();
+// }
+
 #else
 void forceLinkIThreadPoolTests() {}
 #endif

flow/include/flow/IThreadPool.h

@@ -91,7 +91,12 @@ public:
 			sendError(broken_promise());
 	}
 
-	Future<T> getFuture() const { // Call only on the originating thread!
+	// NOTE:
+	// - Call only on the originating thread.
+	// - Must be called before `send` or `sendError`. Will result into crash otherwise, since
+	//   tagAndForward() will take ownership of underlying promise.
+	Future<T> getFuture() const {
+		ASSERT(isValid());
 		return promise.getFuture();
 	}