From c56f875d6e07e194865d8756032fb40f242058e0 Mon Sep 17 00:00:00 2001 From: Jianmin Zhao Date: Wed, 8 Nov 2023 15:33:21 -0800 Subject: [PATCH 1/2] Do not capture the backtrace for OutOfRange exception. As the iterator moves beyond the end of the array or dict, the implementation will throw FleeceException with error code OutOfRange. This is necessary to handle the damaged iterator. However, we don't need to capture the backtrace in this case, which is very expensive as shown in a customer's performance profile, c.f. CBL 5044. --- Fleece/Support/FleeceException.cc | 7 +- Tests/ValueTests.cc | 106 ++++++++++++++++++++++++++++ Xcode/xcconfigs/FleeceTest.xcconfig | 2 + 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/Fleece/Support/FleeceException.cc b/Fleece/Support/FleeceException.cc index 5d6ce765..f1216941 100644 --- a/Fleece/Support/FleeceException.cc +++ b/Fleece/Support/FleeceException.cc @@ -48,8 +48,11 @@ namespace fleece { :std::runtime_error(what) ,code(code_) ,err_no(errno_) - ,backtrace(Backtrace::capture(2)) - { } + { + if (code_ != OutOfRange) { + backtrace = Backtrace::capture(2); + } + } __cold diff --git a/Tests/ValueTests.cc b/Tests/ValueTests.cc index 4c29d3d0..34745e9a 100644 --- a/Tests/ValueTests.cc +++ b/Tests/ValueTests.cc @@ -14,6 +14,7 @@ #define NOMINMAX #endif +#include "fleece/Fleece.h" #include "FleeceTests.hh" #include "Value.hh" #include "Pointer.hh" @@ -232,4 +233,109 @@ namespace fleece { docs.push_back(Doc::fromJSON("[]")); } } + + TEST_CASE("Array Iterators") { + FLDoc doc = nullptr; + SECTION("Empty Array") { + doc = FLDoc_FromJSON("[]"_sl, nullptr); + } + SECTION("Non Empty Array") { + doc = FLDoc_FromJSON("[1]"_sl, nullptr); + } + REQUIRE(doc); + + FLValue val = FLDoc_GetRoot(doc); + FLArray arr = FLValue_AsArray(val); + REQUIRE(arr); + Array::iterator iter = Array::iterator((Array*)arr); + + bool caughtException = false; + bool capturedBacktrace = true; + + // No exception with typical loop. + try { + for (; iter; ++iter); + } catch (...) { + caughtException = true; + } + CHECK(!caughtException); + + caughtException = false; + // ++iter will throw if already at the end. + // OutOfRange exception should contain the backtrace. + try { + ++iter; + } catch (const FleeceException& exc) { + if (exc.code == (int)kFLOutOfRange) { + caughtException = true; + capturedBacktrace = !!exc.backtrace; + } + } + CHECK((caughtException && !capturedBacktrace)); + + // FL Itarator + FLArrayIterator flIter; + FLArrayIterator_Begin(arr, &flIter); + FLValue value; + while (NULL != (value = FLArrayIterator_GetValue(&flIter))) { + FLArrayIterator_Next(&flIter); + } + // Calling Next is okay. It will trigger exception but we won't try to capture the backtrace. + CHECK(!FLArrayIterator_Next(&flIter)); + + FLDoc_Release(doc); + } + + TEST_CASE("Dict Iterators") { + FLDoc doc = nullptr; + SECTION("Empty Dict") { + doc = FLDoc_FromJSON("{}"_sl, nullptr); + } + SECTION("Non Empty Dict") { + doc = FLDoc_FromJSON(R"({"key": 1})"_sl, nullptr); + } + REQUIRE(doc); + + FLValue val = FLDoc_GetRoot(doc); + FLDict dict = FLValue_AsDict(val); + REQUIRE(dict); + Dict::iterator iter = Dict::iterator((Dict*)dict); + + bool caughtException = false; + bool capturedBacktrace = true; + + // No exception with typical loop pattern. + try { + for (; iter; ++iter); + } catch (...) { + caughtException = true; + } + CHECK(!caughtException); + + caughtException = false; + // ++iter will throw if already at the end. + // OutOfRange exception should contain the backtrace. + try { + ++iter; + } catch (const FleeceException& exc) { + if (exc.code == (int)kFLOutOfRange) { + caughtException = true; + capturedBacktrace = !!exc.backtrace; + } + } + CHECK((caughtException && !capturedBacktrace)); + + // FL Itarator + FLDictIterator flIter; + FLDictIterator_Begin(dict, &flIter); + FLValue value; + while (NULL != (value = FLDictIterator_GetValue(&flIter))) { + FLDictIterator_Next(&flIter); + } + // Calling Next is okay. It will trigger exception but we won't try to capture the backtrace. + // Cannot assert it directly. Internally, it uses Dict::iterator::operator++(). + CHECK(!FLDictIterator_Next(&flIter)); + + FLDoc_Release(doc); + } } diff --git a/Xcode/xcconfigs/FleeceTest.xcconfig b/Xcode/xcconfigs/FleeceTest.xcconfig index 913ac2e8..b2aa4a6d 100644 --- a/Xcode/xcconfigs/FleeceTest.xcconfig +++ b/Xcode/xcconfigs/FleeceTest.xcconfig @@ -18,3 +18,5 @@ MACOSX_DEPLOYMENT_TARGET = 10.14 CLANG_WARN__EXIT_TIME_DESTRUCTORS = NO GCC_PREPROCESSOR_DEFINITIONS = $(inherited) _LIBCPP_DEBUG=0 RUN_CLANG_STATIC_ANALYZER = NO + +CODE_SIGNING_ALLOWED = NO From d9e3f33863c8af46f540359f5986335fc714dced Mon Sep 17 00:00:00 2001 From: Jianmin Zhao Date: Fri, 10 Nov 2023 09:36:31 -0800 Subject: [PATCH 2/2] Add some comments per review feedbacks. --- Fleece/Support/FleeceException.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Fleece/Support/FleeceException.cc b/Fleece/Support/FleeceException.cc index f1216941..036117ee 100644 --- a/Fleece/Support/FleeceException.cc +++ b/Fleece/Support/FleeceException.cc @@ -49,6 +49,12 @@ namespace fleece { ,code(code_) ,err_no(errno_) { + // The Fleece iterator (of array and dict) throws OutOfRange exception as one moves + // the iterator, operator++ or FLArrayIterator_Next, for instances, beyond the + // end of the array. Although the exception is caught inside Fleece and the + // client gets return of false, the effort to capture the backtrace can be + // very significant as shown in performance profilings. And, the backtrace is not + // used anyway. if (code_ != OutOfRange) { backtrace = Backtrace::capture(2); }