Skip to content

Commit

Permalink
Further improve AsyncGenerator symmetric transfer test
Browse files Browse the repository at this point in the history
Summary:
(1) ispeters suggested directly comparing stack pointers between generator iterations, which gives a cleaner / faster failure.

(2) Per yfeldblum, let's use folly's pthread compatibility shim to explicitly set the stack size, instead of relying on whatever the ambient environment is. Specifically, the primary thread on Mac can have 8MiB of stack as opposed to 512KIB, which suggests that the previous iteration of D61926452 "worked" to detect broken symmetric transfer by a mechanism other than stack overflow. Given a bad complier + code combination, this fails faster and cleaner now, even with the check from (1) commented out.

Using `CHECK_EQ` instead of `EXPECT_EQ` in the thread because on an older Mac toolchain, letting an exception fly doesn't promptly terminate the test process.

Reviewed By: yfeldblum

Differential Revision: D61930597

fbshipit-source-id: d8ddd2091fc8d8a2b4d023459fd89145d07df597
  • Loading branch information
snarkmaster authored and facebook-github-bot committed Aug 28, 2024
1 parent 4663a80 commit a2be2eb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
53 changes: 41 additions & 12 deletions folly/coro/test/AsyncGeneratorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <folly/futures/Future.h>

#include <folly/portability/GTest.h>
#include <folly/portability/PThread.h>

#include <chrono>
#include <map>
Expand Down Expand Up @@ -466,23 +467,51 @@ TEST_F(AsyncGeneratorTest, BlockingWaitOnThrowingFinalNextDoesNotDeadlock) {
}
}

folly::coro::AsyncGenerator<int> range(int from, int to) {
for (int i = from; i < to; ++i) {
folly::coro::AsyncGenerator<size_t> sizeRange(size_t from, size_t to) {
for (size_t i = from; i < to; ++i) {
co_yield i;
}
}

TEST_F(AsyncGeneratorTest, SymmetricTransfer) {
folly::coro::blockingWait([]() -> folly::coro::Task<void> {
// Large enough to overflow Mac's 512KiB thread stack @ 4 bytes per item
int max = 1000000;
auto g = range(1, max + 1);
long long sum = 0;
while (auto result = co_await g.next()) {
sum += *result;
}
EXPECT_EQ(((long long)max + 1) * max / 2, sum);
}());
// 512KiB is larger than we need, technically. It comes from MacOS's
// default stack size for secondary threads. The superstition behind
// using a realistic size is that even if someone stubs out `setstacksize`
// to be a no-op for a platform, we still have a chance of catching bugs.
constexpr size_t kStackSize = 1 << 19;
auto testFn = [](void* numItersPtr) -> void* {
size_t numIters = *reinterpret_cast<size_t*>(numItersPtr);
size_t sum = 0;
folly::coro::blockingWait([&]() -> folly::coro::Task<void> {
auto g = sizeRange(1, numIters + 1);
int* prevAddress = nullptr;
while (auto result = co_await g.next()) {
[[maybe_unused]] int stackVariable = 0;
if (!prevAddress) {
prevAddress = &stackVariable;
} else {
// If symmetric transfer is broken, this should fire before we
// overflow the stack. `EXPECT_EQ` hangs on Mac / Rosetta.
CHECK_EQ(prevAddress, &stackVariable);
}
sum += *result;
}
CHECK_EQ((numIters + 1) * numIters / 2, sum);
}());
static_assert(sizeof(size_t) <= sizeof(void*));
return reinterpret_cast<void*>(sum);
};
pthread_attr_t attr;
ASSERT_EQ(0, pthread_attr_init(&attr));
ASSERT_EQ(0, pthread_attr_setstacksize(&attr, kStackSize));
pthread_t thread;
// The goal is to overflow the stack with even 1 byte per item
size_t numIters = kStackSize + 1;
ASSERT_EQ(0, pthread_create(&thread, &attr, testFn, &numIters));
void* sumAsPtr;
ASSERT_EQ(0, pthread_join(thread, &sumAsPtr));
// Redundant with above, but ensures the thread actually ran!
EXPECT_EQ((numIters + 1) * numIters / 2, reinterpret_cast<size_t>(sumAsPtr));
}

TEST(AsyncGenerator, YieldCoError) {
Expand Down
1 change: 1 addition & 0 deletions folly/coro/test/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ cpp_unittest(
"//folly/experimental/coro:with_cancellation",
"//folly/futures:core",
"//folly/portability:gtest",
"//folly/portability:pthread",
],
)

Expand Down

0 comments on commit a2be2eb

Please sign in to comment.