diff options
author | Kevin Pulo <kevin.pulo@mongodb.com> | 2020-10-26 17:11:18 +1100 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-11-02 09:58:44 +0000 |
commit | 7d8e64df2d2d56a821f638ef88aa619403d03d31 (patch) | |
tree | 6ada2d481c56b9754ec7848caf146cd94149148f /src | |
parent | 4d2dea00415bf02d2b32d0474c93d251ce6568cc (diff) | |
download | mongo-7d8e64df2d2d56a821f638ef88aa619403d03d31.tar.gz |
SERVER-44570 Add tripwire assertions (tassert)
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/commands/server_status.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/logical_time_metadata_hook.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/query/index_bounds_builder.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/query/index_tag.cpp | 2 | ||||
-rw-r--r-- | src/mongo/s/query/cluster_aggregation_planner.cpp | 4 | ||||
-rw-r--r-- | src/mongo/s/query/cluster_find.cpp | 4 | ||||
-rw-r--r-- | src/mongo/unittest/unittest.h | 14 | ||||
-rw-r--r-- | src/mongo/unittest/unittest_main.cpp | 2 | ||||
-rw-r--r-- | src/mongo/util/assert_util.cpp | 34 | ||||
-rw-r--r-- | src/mongo/util/assert_util.h | 60 | ||||
-rw-r--r-- | src/mongo/util/assert_util_test.cpp | 92 | ||||
-rw-r--r-- | src/mongo/util/quick_exit.cpp | 2 | ||||
-rw-r--r-- | src/mongo/util/quick_exit.h | 17 |
13 files changed, 229 insertions, 11 deletions
diff --git a/src/mongo/db/commands/server_status.cpp b/src/mongo/db/commands/server_status.cpp index 4fc7aeba20a..7a4af634ad8 100644 --- a/src/mongo/db/commands/server_status.cpp +++ b/src/mongo/db/commands/server_status.cpp @@ -253,6 +253,7 @@ public: asserts.append("warning", assertionCount.warning.loadRelaxed()); asserts.append("msg", assertionCount.msg.loadRelaxed()); asserts.append("user", assertionCount.user.loadRelaxed()); + asserts.append("tripwire", assertionCount.tripwire.loadRelaxed()); asserts.append("rollovers", assertionCount.rollovers.loadRelaxed()); return asserts.obj(); } diff --git a/src/mongo/db/logical_time_metadata_hook.cpp b/src/mongo/db/logical_time_metadata_hook.cpp index 11a57a59885..a2019f5ee44 100644 --- a/src/mongo/db/logical_time_metadata_hook.cpp +++ b/src/mongo/db/logical_time_metadata_hook.cpp @@ -64,7 +64,9 @@ Status LogicalTimeMetadataHook::readReplyMetadata(OperationContext* opCtx, auto timeTracker = OperationTimeTracker::get(opCtx); auto operationTime = metadataObj[kOperationTimeFieldName]; if (!operationTime.eoo()) { - invariant(operationTime.type() == BSONType::bsonTimestamp); + tassert(4457010, + "operationTime must be a timestamp if present", + operationTime.type() == BSONType::bsonTimestamp); timeTracker->updateOperationTime(LogicalTime(operationTime.timestamp())); } } diff --git a/src/mongo/db/query/index_bounds_builder.cpp b/src/mongo/db/query/index_bounds_builder.cpp index 54f2846cdc0..e423f60844c 100644 --- a/src/mongo/db/query/index_bounds_builder.cpp +++ b/src/mongo/db/query/index_bounds_builder.cpp @@ -574,7 +574,9 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr, // values. One exception is for collation, whose index bounds are tracked as INEXACT_FETCH, // but only because the index data is different than the user data, not because the range // is imprecise. - invariant(*tightnessOut == IndexBoundsBuilder::EXACT || index.collator); + tassert(4457011, + "Cannot invert inexact bounds", + *tightnessOut == IndexBoundsBuilder::EXACT || index.collator); // If the index is multikey on this path, it doesn't matter what the tightness of the child // is, we must return INEXACT_FETCH. Consider a multikey index on 'a' with document diff --git a/src/mongo/db/query/index_tag.cpp b/src/mongo/db/query/index_tag.cpp index d14be1444ea..93e5cdb7402 100644 --- a/src/mongo/db/query/index_tag.cpp +++ b/src/mongo/db/query/index_tag.cpp @@ -228,7 +228,7 @@ bool pushdownNode(MatchExpression* node, return pushdownNode(node, indexedOr, std::move(destinations)); } - MONGO_UNREACHABLE; + MONGO_UNREACHABLE_TASSERT(4457014); } // Populates 'out' with all descendants of 'node' that have OrPushdownTags, assuming the initial diff --git a/src/mongo/s/query/cluster_aggregation_planner.cpp b/src/mongo/s/query/cluster_aggregation_planner.cpp index 3a703dba1f7..71b1bddaac8 100644 --- a/src/mongo/s/query/cluster_aggregation_planner.cpp +++ b/src/mongo/s/query/cluster_aggregation_planner.cpp @@ -723,7 +723,9 @@ Status dispatchPipelineAndMerge(OperationContext* opCtx, // If we sent the entire pipeline to a single shard, store the remote cursor and return. if (!shardDispatchResults.splitPipeline) { - invariant(shardDispatchResults.remoteCursors.size() == 1); + tassert(4457012, + "pipeline was split, but more than one remote cursor is present", + shardDispatchResults.remoteCursors.size() == 1); auto&& remoteCursor = std::move(shardDispatchResults.remoteCursors.front()); const auto shardId = remoteCursor->getShardId().toString(); const auto reply = uassertStatusOK(storePossibleCursor(opCtx, diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp index cc81dd0df0c..de9b1fa144f 100644 --- a/src/mongo/s/query/cluster_find.cpp +++ b/src/mongo/s/query/cluster_find.cpp @@ -299,7 +299,9 @@ CursorId runQueryWithoutRetrying(OperationContext* opCtx, } // Tailable cursors can't have a sort, which should have already been validated. - invariant(sortComparatorObj.isEmpty() || !query.getQueryRequest().isTailable()); + tassert(4457013, + "tailable cursor unexpectedly has a sort", + sortComparatorObj.isEmpty() || !query.getQueryRequest().isTailable()); // Construct the requests that we will use to establish cursors on the targeted shards, // attaching the shardVersion and txnNumber, if necessary. diff --git a/src/mongo/unittest/unittest.h b/src/mongo/unittest/unittest.h index 160532e4528..2ef1d7cd77e 100644 --- a/src/mongo/unittest/unittest.h +++ b/src/mongo/unittest/unittest.h @@ -506,12 +506,22 @@ public: /** * Called on the test object before running the test. */ - virtual void setUp() {} + virtual void setUp() { + // React to any tasserts in the unittest framework initialisation, or between tests. + checkForTripwireAssertions(); + // Clear tasserts for the code that's about to be under test. + assertionCount.tripwire.store(0); + } /** * Called on the test object after running the test. */ - virtual void tearDown() {} + virtual void tearDown() { + // React to any tasserts in the code that was under test. + checkForTripwireAssertions(); + // Clear tasserts in case of any between tests, or during the unittest framework shutdown. + assertionCount.tripwire.store(0); + } protected: /** diff --git a/src/mongo/unittest/unittest_main.cpp b/src/mongo/unittest/unittest_main.cpp index b1286ed772a..1056198a04f 100644 --- a/src/mongo/unittest/unittest_main.cpp +++ b/src/mongo/unittest/unittest_main.cpp @@ -127,5 +127,7 @@ int main(int argc, char** argv) { std::cerr << "Global deinitilization failed: " << ret.reason() << std::endl; } + ::mongo::checkForTripwireAssertions(); + return result; } diff --git a/src/mongo/util/assert_util.cpp b/src/mongo/util/assert_util.cpp index d1e9281f490..7039a5c1162 100644 --- a/src/mongo/util/assert_util.cpp +++ b/src/mongo/util/assert_util.cpp @@ -276,6 +276,40 @@ void internalAssertWithLocation(SourceLocationHolder loc, const Status& status) error_details::throwExceptionForStatus(status); } +void tassertFailedWithLocation(SourceLocationHolder loc, const Status& status) { + assertionCount.condrollover(assertionCount.tripwire.addAndFetch(1)); + LOGV2(4457000, "Tripwire assertion", "error"_attr = status, "location"_attr = loc); + breakpoint(); + error_details::throwExceptionForStatus(status); +} + +void tassertWithLocation(SourceLocationHolder loc, const Status& status) { + if (status.isOK()) + return; + tassertFailedWithLocation(std::move(loc), status); +} + +void checkForTripwireAssertions(int code) { + auto tripwireOccurrences = assertionCount.tripwire.load(); + if (tripwireOccurrences == 0) { + return; + } + if (code == EXIT_CLEAN) { + LOGV2_FATAL_NOTRACE( + 4457001, + "Aborting process during clean exit due to prior failed tripwire assertions, " + "please check your logs for \"Tripwire assertion\" entries with log id 4457000.", + "occurrences"_attr = tripwireOccurrences, + "exitCode"_attr = code); + } else { + LOGV2(4457002, + "Detected prior failed tripwire assertions during unclean exit, please check your " + "logs for \"Tripwire assertion\" entries with log id 4457000.", + "occurrences"_attr = tripwireOccurrences, + "exitCode"_attr = code); + } +} + std::string causedBy(StringData e) { constexpr auto prefix = " :: caused by :: "_sd; std::string out; diff --git a/src/mongo/util/assert_util.h b/src/mongo/util/assert_util.h index f1e54d2a7ce..0e2558cf093 100644 --- a/src/mongo/util/assert_util.h +++ b/src/mongo/util/assert_util.h @@ -39,6 +39,7 @@ #include "mongo/platform/source_location.h" #include "mongo/util/concurrency/thread_name.h" #include "mongo/util/debug_util.h" +#include "mongo/util/exit_code.h" #define MONGO_INCLUDE_INVARIANT_H_WHITELISTED #include "mongo/util/invariant.h" @@ -56,6 +57,7 @@ public: AtomicWord<int> warning; AtomicWord<int> msg; AtomicWord<int> user; + AtomicWord<int> tripwire; AtomicWord<int> rollovers; }; @@ -472,6 +474,59 @@ inline void internalAssertWithLocation(SourceLocationHolder loc, StatusWith<T>&& } /** + * "tripwire/test assert". Like uassert, but with a deferred-fatality tripwire that gets + * checked prior to normal shutdown. Used to ensure that this assertion will both fail the + * operation and also cause a test suite failure. + */ +#define tassert(...) ::mongo::tassertWithLocation(MONGO_SOURCE_LOCATION(), __VA_ARGS__) + +MONGO_COMPILER_NORETURN void tassertFailedWithLocation(SourceLocationHolder loc, + const Status& status); + +#define tasserted(...) ::mongo::tassertFailedWithLocation(MONGO_SOURCE_LOCATION(), __VA_ARGS__) + +MONGO_COMPILER_NORETURN inline void tassertFailedWithLocation(SourceLocationHolder loc, + int msgid, + const std::string& msg) { + tassertFailedWithLocation(std::move(loc), Status(ErrorCodes::Error(msgid), msg)); +} + +void tassertWithLocation(SourceLocationHolder loc, const Status& status); + +inline void tassertWithLocation(SourceLocationHolder loc, Status&& status) { + tassertWithLocation(std::move(loc), status); +} + +inline void tassertWithLocation(SourceLocationHolder loc, + int msgid, + const std::string& msg, + bool expr) { + if (MONGO_unlikely(!expr)) + tassertWithLocation(std::move(loc), Status(ErrorCodes::Error(msgid), msg)); +} + +template <typename T> +inline void tassertWithLocation(SourceLocationHolder loc, const StatusWith<T>& sw) { + tassertWithLocation(std::move(loc), sw.getStatus()); +} + +template <typename T> +inline void tassertWithLocation(SourceLocationHolder loc, StatusWith<T>&& sw) { + tassertWithLocation(std::move(loc), sw); +} + +/** + * Handle tassert failures during exit with a given exit code. + * + * If the exit code is success (ie. EXIT_CLEAN, EXIT_SUCCESS, or 0) and there have been any tassert + * failures, then abort the process. + * + * Otherwise, if the exit code is an error, then just log the number of tassert failures (and then + * continue exiting with that exit code). + */ +void checkForTripwireAssertions(int code = EXIT_CLEAN); + +/** * verify is deprecated. It is like invariant() in debug builds and massert() in release builds. */ #define verify(expression) MONGO_verify(expression) @@ -642,3 +697,8 @@ Status exceptionToStatus() noexcept; */ #define MONGO_UNREACHABLE ::mongo::invariantFailed("Hit a MONGO_UNREACHABLE!", __FILE__, __LINE__); + +#define MONGO_UNREACHABLE_TASSERT(msgid) \ + ::mongo::tassertFailedWithLocation( \ + MONGO_SOURCE_LOCATION(), \ + Status(ErrorCodes::Error(msgid), "Hit a MONGO_UNREACHABLE_TASSERT!")); diff --git a/src/mongo/util/assert_util_test.cpp b/src/mongo/util/assert_util_test.cpp index 0db644dd744..bb5a502a65d 100644 --- a/src/mongo/util/assert_util_test.cpp +++ b/src/mongo/util/assert_util_test.cpp @@ -38,6 +38,7 @@ #include "mongo/unittest/death_test.h" #include "mongo/unittest/unittest.h" #include "mongo/util/assert_util.h" +#include "mongo/util/quick_exit.h" #include "mongo/util/str.h" namespace mongo { @@ -237,6 +238,9 @@ TEST(AssertUtils, InternalAssertWithStatus) { ASSERT_EQ(ex.code(), ErrorCodes::BadValue); ASSERT_EQ(ex.reason(), "Test"); } + + internalAssert(Status::OK()); + ASSERT_EQ(userAssertions, assertionCount.user.load()); } @@ -273,6 +277,94 @@ TEST(AssertUtils, MassertTypedExtraInfoWorks) { } } +// tassert +namespace { + +void doTassert() { + auto tripwireAssertions = assertionCount.tripwire.load(); + + try { + Status status = {ErrorCodes::BadValue, "Test with Status"}; + tassert(status); + } catch (const DBException& ex) { + ASSERT_EQ(ex.code(), ErrorCodes::BadValue); + ASSERT_EQ(ex.reason(), "Test with Status"); + } + ASSERT_EQ(tripwireAssertions + 1, assertionCount.tripwire.load()); + + tassert(Status::OK()); + ASSERT_EQ(tripwireAssertions + 1, assertionCount.tripwire.load()); + + try { + tassert(4457090, "Test with expression", false); + } catch (const DBException& ex) { + ASSERT_EQ(ex.code(), 4457090); + ASSERT_EQ(ex.reason(), "Test with expression"); + } + ASSERT_EQ(tripwireAssertions + 2, assertionCount.tripwire.load()); + + tassert(4457091, "Another test with expression", true); + ASSERT_EQ(tripwireAssertions + 2, assertionCount.tripwire.load()); + + try { + tasserted(4457092, "Test with tasserted"); + } catch (const DBException& ex) { + ASSERT_EQ(ex.code(), 4457092); + ASSERT_EQ(ex.reason(), "Test with tasserted"); + } + ASSERT_EQ(tripwireAssertions + 3, assertionCount.tripwire.load()); +} + +} // namespace + +DEATH_TEST(TassertTerminationTest, + tassertCleanLogMsg, + "Aborting process during clean exit due to prior failed tripwire assertions") { + doTassert(); +} + +DEATH_TEST(TassertTerminationTest, tassertCleanLogMsgCode, "4457001") { + doTassert(); +} + +DEATH_TEST(TassertTerminationTest, tassertCleanIndividualLogMsg, "Tripwire assertion") { + doTassert(); +} + +DEATH_TEST(TassertTerminationTest, tassertCleanIndividualLogMsgCode, "4457000") { + doTassert(); +} + +DEATH_TEST(TassertTerminationTest, + tassertUncleanLogMsg, + "Detected prior failed tripwire assertions during unclean exit") { + doTassert(); + quickExit(EXIT_ABRUPT); +} + +DEATH_TEST(TassertTerminationTest, tassertUncleanLogMsgCode, "4457002") { + doTassert(); + quickExit(EXIT_ABRUPT); +} + +DEATH_TEST(TassertTerminationTest, tassertUncleanIndividualLogMsg, "Tripwire assertion") { + doTassert(); + quickExit(EXIT_ABRUPT); +} + +DEATH_TEST(TassertTerminationTest, tassertUncleanIndividualLogMsgCode, "4457000") { + doTassert(); + quickExit(EXIT_ABRUPT); +} + +DEATH_TEST(TassertTerminationTest, mongoUnreachableNonFatal, "Hit a MONGO_UNREACHABLE_TASSERT!") { + try { + MONGO_UNREACHABLE_TASSERT(4457093); + } catch (const DBException&) { + // Catch the DBException, to ensure that we eventually abort during clean exit. + } +} + // fassert and its friends DEATH_TEST(FassertionTerminationTest, fassert, "40206") { fassert(40206, false); diff --git a/src/mongo/util/quick_exit.cpp b/src/mongo/util/quick_exit.cpp index 4bb25f931b4..caf113da50a 100644 --- a/src/mongo/util/quick_exit.cpp +++ b/src/mongo/util/quick_exit.cpp @@ -73,7 +73,7 @@ namespace { stdx::mutex* const quickExitMutex = new stdx::mutex; } // namespace -void quickExit(int code) { +void quickExitWithoutLogging(int code) { // Ensure that only one thread invokes the last rites here. No // RAII here - we never want to unlock this. if (quickExitMutex) diff --git a/src/mongo/util/quick_exit.h b/src/mongo/util/quick_exit.h index 4582ae4839f..78dd836472a 100644 --- a/src/mongo/util/quick_exit.h +++ b/src/mongo/util/quick_exit.h @@ -28,17 +28,28 @@ */ #include "mongo/platform/compiler.h" +#include "mongo/util/assert_util.h" namespace mongo { -/** This function will call ::_exit and not return. Use this instead of calling ::_exit +/** The quickExit function will call ::_exit and not return. Use this instead of calling ::_exit * directly: * - It offers a debugger hook to catch the process before leaving code under our control. * - For some builds (leak sanitizer) it gives us an opportunity to dump leaks. * - * The function is named differently than quick_exit so that we can distinguish it from + * The quickExit function is named differently than quick_exit so that we can distinguish it from * the C++11 function of the same name. + * + * The quickExitWithoutLogging function is the same as quickExit, except that it doesn't do any + * pre-exit checks that might result in logging. This explains why quickExit is implemented as an + * inline wrapper around quickExitWithoutLogging - the pre-exit checks and logging need to refer + * to mongo symbols, which aren't permitted in quick_exit.cpp. */ -MONGO_COMPILER_NORETURN void quickExit(int); +MONGO_COMPILER_NORETURN void quickExitWithoutLogging(int); + +MONGO_COMPILER_NORETURN inline void quickExit(int code) { + checkForTripwireAssertions(code); + quickExitWithoutLogging(code); +} } // namespace mongo |