logging: improve the AsyncFileWriter flush test()

Summary:
This test has run into occasional failures on continuous build test runs.
Unfortunately when something goes wrong it crashes in the std::thread
destructor due to this thread still being joinable when it is destroyed, which
hides information about what actually failed in the test.

This updates the test to immediately detach the thread, so that on error we
will be able see the real failure reason.

This also increases the size of the message that we write, which will hopefully
help ensure that this write always blocks.

Reviewed By: wez

Differential Revision: D5295574

fbshipit-source-id: ea8cfa855613398f88f9f982c600ec661018a31c

diff --git a/folly/experimental/logging/test/AsyncFileWriterTest.cpp b/folly/experimental/logging/test/AsyncFileWriterTest.cpp
index 7b03ef5ee229feddddec48791c2844d626ba4d6e..3333c0657b4f45131ae2408fd8184cb20266e361 100644 (file)
--- a/folly/experimental/logging/test/AsyncFileWriterTest.cpp
+++ b/folly/experimental/logging/test/AsyncFileWriterTest.cpp
@@ -208,7 +208,7 @@ TEST(AsyncFileWriter, flush) {
   AsyncFileWriter writer{std::move(writePipe)};
 
   // Write a message
   AsyncFileWriter writer{std::move(writePipe)};
 
   // Write a message

-  writer.writeMessage(std::string{"test message"});
+  writer.writeMessage("test message: " + std::string(200, 'x'));

 
   // Call flush().  Use a separate thread, since this should block until we
   // consume data from the pipe.
 
   // Call flush().  Use a separate thread, since this should block until we
   // consume data from the pipe.


@@ -217,8 +217,16 @@ TEST(AsyncFileWriter, flush) {
   auto flushFunction = [&] { writer.flush(); };
   std::thread flushThread{
       [&]() { promise.setTry(makeTryWith(flushFunction)); }};
   auto flushFunction = [&] { writer.flush(); };
   std::thread flushThread{
       [&]() { promise.setTry(makeTryWith(flushFunction)); }};

+  // Detach the flush thread now rather than joining it at the end of the
+  // function.  This way if something goes wrong during the test we will fail
+  // with the real error, rather than crashing due to the std::thread
+  // destructor running on a still-joinable thread.
+  flushThread.detach();

 
   // Sleep briefly, and make sure flush() still hasn't completed.
 
   // Sleep briefly, and make sure flush() still hasn't completed.

+  // If it has completed this doesn't necessarily indicate a bug in
+  // AsyncFileWriter, but instead indicates that our test code failed to
+  // successfully cause a blocking write.

   /* sleep override */
   std::this_thread::sleep_for(10ms);
   EXPECT_FALSE(future.isReady());
   /* sleep override */
   std::this_thread::sleep_for(10ms);
   EXPECT_FALSE(future.isReady());


@@ -230,7 +238,6 @@ TEST(AsyncFileWriter, flush) {
 
   // Make sure flush completes successfully now
   future.get(10ms);
 
   // Make sure flush completes successfully now
   future.get(10ms);

-  flushThread.join();

 }
 #endif
 
 }
 #endif