From fc9b0aaf4db69c79a1079608bec8f18c8f2cf9f0 Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Mon, 28 Oct 2024 09:45:50 +0000 Subject: [PATCH 1/3] Fixed use after free --- ydb/core/fq/libs/row_dispatcher/json_parser.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ydb/core/fq/libs/row_dispatcher/json_parser.cpp b/ydb/core/fq/libs/row_dispatcher/json_parser.cpp index 56946ef49a38..af24ee071c65 100644 --- a/ydb/core/fq/libs/row_dispatcher/json_parser.cpp +++ b/ydb/core/fq/libs/row_dispatcher/json_parser.cpp @@ -264,7 +264,7 @@ class TJsonParser::TImpl { public: TImpl(const TVector& columns, const TVector& types, ui64 batchSize, TDuration batchCreationTimeout) : Alloc(__LOCATION__, NKikimr::TAlignedPagePoolCounters(), true, false) - , TypeEnv(Alloc) + , TypeEnv(std::make_unique(Alloc)) , BatchSize(batchSize) , BatchCreationTimeout(batchCreationTimeout) , ParsedValues(columns.size()) @@ -273,7 +273,7 @@ class TJsonParser::TImpl { with_lock (Alloc) { auto functonRegistry = NKikimr::NMiniKQL::CreateFunctionRegistry(&PrintBackTrace, NKikimr::NMiniKQL::CreateBuiltinRegistry(), false, {}); - NKikimr::NMiniKQL::TProgramBuilder programBuilder(TypeEnv, *functonRegistry); + NKikimr::NMiniKQL::TProgramBuilder programBuilder(*TypeEnv, *functonRegistry); Columns.reserve(columns.size()); for (size_t i = 0; i < columns.size(); i++) { @@ -370,8 +370,11 @@ class TJsonParser::TImpl { } ~TImpl() { - Alloc.Acquire(); - ClearColumns(0); + with_lock (Alloc) { + ClearColumns(0); + Columns.clear(); + TypeEnv.reset(); + } } private: @@ -392,7 +395,7 @@ class TJsonParser::TImpl { private: NKikimr::NMiniKQL::TScopedAlloc Alloc; - NKikimr::NMiniKQL::TTypeEnvironment TypeEnv; + std::unique_ptr TypeEnv; const ui64 BatchSize; const TDuration BatchCreationTimeout; @@ -402,7 +405,7 @@ class TJsonParser::TImpl { TJsonParserBuffer Buffer; simdjson::ondemand::parser Parser; - TVector>> ParsedValues; + TVector ParsedValues; }; TJsonParser::TJsonParser(const TVector& columns, const TVector& types, ui64 batchSize, TDuration batchCreationTimeout) From 891ed8f3064dc8eb1656880f2bec804bbfb62eb3 Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Mon, 28 Oct 2024 16:07:45 +0000 Subject: [PATCH 2/3] Fixed ParsedValues clearing --- ydb/core/fq/libs/row_dispatcher/json_parser.cpp | 1 + ydb/core/fq/libs/row_dispatcher/ut/json_parser_ut.cpp | 11 +++++++++++ ydb/core/fq/libs/row_dispatcher/ut/ya.make | 3 --- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ydb/core/fq/libs/row_dispatcher/json_parser.cpp b/ydb/core/fq/libs/row_dispatcher/json_parser.cpp index af24ee071c65..03c49820bbb7 100644 --- a/ydb/core/fq/libs/row_dispatcher/json_parser.cpp +++ b/ydb/core/fq/libs/row_dispatcher/json_parser.cpp @@ -372,6 +372,7 @@ class TJsonParser::TImpl { ~TImpl() { with_lock (Alloc) { ClearColumns(0); + ParsedValues.clear(); Columns.clear(); TypeEnv.reset(); } diff --git a/ydb/core/fq/libs/row_dispatcher/ut/json_parser_ut.cpp b/ydb/core/fq/libs/row_dispatcher/ut/json_parser_ut.cpp index bdf74270094d..7ffbe7e27d91 100644 --- a/ydb/core/fq/libs/row_dispatcher/ut/json_parser_ut.cpp +++ b/ydb/core/fq/libs/row_dispatcher/ut/json_parser_ut.cpp @@ -1,3 +1,5 @@ +#include + #include #include @@ -22,7 +24,16 @@ class TFixture : public NUnitTest::TBaseFixture { TFixture() : Runtime(true) {} + static void SegmentationFaultHandler(int) { + Cerr << "segmentation fault call stack:" << Endl; + FormatBackTrace(&Cerr); + abort(); + } + void SetUp(NUnitTest::TTestContext&) override { + NKikimr::EnableYDBBacktraceFormat(); + signal(SIGSEGV, &SegmentationFaultHandler); + TAutoPtr app = new TAppPrepare(); Runtime.SetLogBackend(CreateStderrBackend()); Runtime.SetLogPriority(NKikimrServices::FQ_ROW_DISPATCHER, NLog::PRI_TRACE); diff --git a/ydb/core/fq/libs/row_dispatcher/ut/ya.make b/ydb/core/fq/libs/row_dispatcher/ut/ya.make index 25242d092f28..1e6700f07b93 100644 --- a/ydb/core/fq/libs/row_dispatcher/ut/ya.make +++ b/ydb/core/fq/libs/row_dispatcher/ut/ya.make @@ -16,11 +16,8 @@ PEERDIR( ydb/core/fq/libs/row_dispatcher ydb/core/testlib ydb/core/testlib/actors - ydb/library/yql/udfs/common/json2 - ydb/library/yql/udfs/common/yson2 ydb/tests/fq/pq_async_io ydb/library/yql/sql/pg_dummy - ydb/library/yql/udfs/common/clickhouse/client ) SIZE(MEDIUM) From d96f3a70df40d9700dc3530f6cce2dcf49703726 Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Mon, 28 Oct 2024 16:09:45 +0000 Subject: [PATCH 3/3] Fixed failed to optimize error --- ydb/core/fq/libs/row_dispatcher/ut/ya.make | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ydb/core/fq/libs/row_dispatcher/ut/ya.make b/ydb/core/fq/libs/row_dispatcher/ut/ya.make index 1e6700f07b93..bb66ec57798f 100644 --- a/ydb/core/fq/libs/row_dispatcher/ut/ya.make +++ b/ydb/core/fq/libs/row_dispatcher/ut/ya.make @@ -16,6 +16,8 @@ PEERDIR( ydb/core/fq/libs/row_dispatcher ydb/core/testlib ydb/core/testlib/actors + ydb/library/yql/udfs/common/json2 + ydb/library/yql/udfs/common/yson2 ydb/tests/fq/pq_async_io ydb/library/yql/sql/pg_dummy )