Commit b2a27c9d authored by Zeyi (Rice) Fan's avatar Zeyi (Rice) Fan Committed by Facebook Github Bot

check if subclass of AsyncLogWriter called cleanup

Summary: This diff adds a check in `AsyncLogWriter`'s destructor that checks if its subclass called `cleanup()` to drain all the pending log messages in the queue during destructing.

Reviewed By: yfeldblum

Differential Revision: D15383734

fbshipit-source-id: deca9bdc84d5f514f2d47a250d31a68126d20aad
parent ffdc8a9a
......@@ -18,6 +18,7 @@
#include <folly/Exception.h>
#include <folly/FileUtil.h>
#include <folly/detail/AtFork.h>
#include <folly/logging/LoggerDB.h>
#include <folly/system/ThreadName.h>
using folly::File;
......@@ -45,6 +46,23 @@ AsyncLogWriter::AsyncLogWriter() {
}
}
AsyncLogWriter::~AsyncLogWriter() {
{
auto data = data_.lock();
if (!(data->flags & FLAG_DESTROYING)) {
LoggerDB::internalWarning(
__FILE__, __LINE__, "cleanup() is not called before destroying");
stopIoThread(data, FLAG_DESTROYING);
assert(false);
}
}
// Unregister the atfork handler after stopping the I/O thread.
// preFork(), postForkParent(), and postForkChild() calls can run
// concurrently with the destructor until unregisterHandler() returns.
folly::detail::AtFork::unregisterHandler(this);
}
void AsyncLogWriter::cleanup() {
std::vector<std::string>* ioQueue;
size_t numDiscarded;
......@@ -60,11 +78,6 @@ void AsyncLogWriter::cleanup() {
numDiscarded = data->numDiscarded;
}
// Unregister the atfork handler after stopping the I/O thread.
// preFork(), postForkParent(), and postForkChild() calls can run
// concurrently with the destructor until unregisterHandler() returns.
folly::detail::AtFork::unregisterHandler(this);
// If there are still any pending messages, flush them now.
if (!ioQueue->empty()) {
performIO(ioQueue, numDiscarded);
......
......@@ -56,6 +56,8 @@ class AsyncLogWriter : public LogWriter {
explicit AsyncLogWriter();
virtual ~AsyncLogWriter() override;
void writeMessage(folly::StringPiece buffer, uint32_t flags = 0) override;
void writeMessage(std::string&& buffer, uint32_t flags = 0) override;
......
/*
* Copyright 2017-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <folly/logging/AsyncLogWriter.h>
#include <folly/logging/LoggerDB.h>
#include <folly/portability/GMock.h>
#include <folly/portability/GTest.h>
#include <folly/test/TestUtils.h>
#include <iostream>
using namespace folly;
using namespace std::literals::chrono_literals;
using testing::ContainsRegex;
namespace {
static bool* expectedMessage;
void handleLoggingError(
StringPiece /* file */,
int /* lineNumber */,
std::string&& msg) {
if (folly::kIsDebug) {
std::cerr << msg << std::endl;
} else {
*expectedMessage = (msg == "cleanup() is not called before destroying");
}
}
class NoCleanUpLogWriter : public AsyncLogWriter {
void performIO(std::vector<std::string>*, size_t) override {}
bool ttyOutput() const override {
return false;
}
};
} // namespace
TEST(AsyncLogWriterDeathTest, cleanupWarning) {
bool flag;
expectedMessage = &flag;
LoggerDB::setInternalWarningHandler(handleLoggingError);
if (folly::kIsDebug) {
EXPECT_DEATH(
{ NoCleanUpLogWriter{}; },
"cleanup\\(\\) is not called before destroying");
} else {
{ NoCleanUpLogWriter{}; }
EXPECT_TRUE(flag);
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment