Skip to content

Commit

Permalink
Fix memory leak due to a misuse of AWS SDK (#9810)
Browse files Browse the repository at this point in the history
  • Loading branch information
jepett0 authored Sep 27, 2024
1 parent 82ee538 commit f4c79be
Show file tree
Hide file tree
Showing 26 changed files with 751 additions and 373 deletions.
36 changes: 36 additions & 0 deletions ydb/core/driver_lib/run/kikimr_services_initializers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,29 @@

#include <util/system/hostname.h>

#ifndef KIKIMR_DISABLE_S3_OPS
#include <aws/core/Aws.h>
#endif

namespace {

#ifndef KIKIMR_DISABLE_S3_OPS
struct TAwsApiGuard {
TAwsApiGuard() {
Aws::InitAPI(Options);
}

~TAwsApiGuard() {
Aws::ShutdownAPI(Options);
}

private:
Aws::SDKOptions Options;
};
#endif

}

namespace NKikimr {

namespace NKikimrServicesInitializers {
Expand Down Expand Up @@ -2816,5 +2839,18 @@ void TGraphServiceInitializer::InitializeServices(NActors::TActorSystemSetup* se
TActorSetupCmd(NGraph::CreateGraphService(appData->TenantName), TMailboxType::HTSwap, appData->UserPoolId));
}

#ifndef KIKIMR_DISABLE_S3_OPS
TAwsApiInitializer::TAwsApiInitializer(IGlobalObjectStorage& globalObjects)
: GlobalObjects(globalObjects)
{
}

void TAwsApiInitializer::InitializeServices(NActors::TActorSystemSetup* setup, const NKikimr::TAppData* appData) {
Y_UNUSED(setup);
Y_UNUSED(appData);
GlobalObjects.AddGlobalObject(std::make_shared<TAwsApiGuard>());
}
#endif

} // namespace NKikimrServicesInitializers
} // namespace NKikimr
11 changes: 11 additions & 0 deletions ydb/core/driver_lib/run/kikimr_services_initializers.h
Original file line number Diff line number Diff line change
Expand Up @@ -618,5 +618,16 @@ class TGraphServiceInitializer : public IKikimrServicesInitializer {
void InitializeServices(NActors::TActorSystemSetup* setup, const NKikimr::TAppData* appData) override;
};

#ifndef KIKIMR_DISABLE_S3_OPS
class TAwsApiInitializer : public IServiceInitializer {
IGlobalObjectStorage& GlobalObjects;

public:
TAwsApiInitializer(IGlobalObjectStorage& globalObjects);

void InitializeServices(NActors::TActorSystemSetup* setup, const NKikimr::TAppData* appData) override;
};
#endif

} // namespace NKikimrServicesInitializers
} // namespace NKikimr
6 changes: 6 additions & 0 deletions ydb/core/driver_lib/run/run.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,12 @@ TIntrusivePtr<TServiceInitializersList> TKikimrRunner::CreateServiceInitializers
sil->AddServiceInitializer(new TGraphServiceInitializer(runConfig));
}

#ifndef KIKIMR_DISABLE_S3_OPS
if (serviceMask.EnableAwsService) {
sil->AddServiceInitializer(new TAwsApiInitializer(*this));
}
#endif

return sil;
}

Expand Down
1 change: 1 addition & 0 deletions ydb/core/driver_lib/run/service_mask.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ union TBasicKikimrServicesMask {
bool EnableGraphService:1;
bool EnableCompDiskLimiter:1;
bool EnableGroupedMemoryLimiter:1;
bool EnableAwsService:1;
};

struct {
Expand Down
10 changes: 10 additions & 0 deletions ydb/core/driver_lib/run/ya.make
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
LIBRARY(run)

IF (OS_WINDOWS)
CFLAGS(
-DKIKIMR_DISABLE_S3_OPS
)
ELSE()
PEERDIR(
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
)
ENDIF()

SRCS(
auto_config_initializer.cpp
config.cpp
Expand Down
13 changes: 13 additions & 0 deletions ydb/core/tx/columnshard/ut_schema/ut_columnshard_schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

#include <util/system/hostname.h>
#include <library/cpp/deprecated/atomic/atomic.h>
#include <library/cpp/testing/hook/hook.h>

#include <aws/core/Aws.h>

namespace NKikimr {

Expand All @@ -32,6 +35,16 @@ enum class EInitialEviction {

namespace {

Aws::SDKOptions Options;

Y_TEST_HOOK_BEFORE_RUN(InitAwsAPI) {
Aws::InitAPI(Options);
}

Y_TEST_HOOK_AFTER_RUN(ShutdownAwsAPI) {
Aws::ShutdownAPI(Options);
}

static const std::vector<NArrow::NTest::TTestColumn> testYdbSchema = TTestSchema::YdbSchema();
static const std::vector<NArrow::NTest::TTestColumn> testYdbPk = TTestSchema::YdbPkSchema();

Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/columnshard/ut_schema/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ PEERDIR(
library/cpp/getopt
library/cpp/regex/pcre
library/cpp/svnversion
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
ydb/core/testlib/default
ydb/core/tx/columnshard/hooks/abstract
ydb/core/tx/columnshard/hooks/testing
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/datashard/import_s3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <ydb/core/tablet/resource_broker.h>
#include <ydb/core/wrappers/s3_wrapper.h>
#include <ydb/core/wrappers/s3_storage.h>
#include <ydb/core/wrappers/s3_storage_config.h>
#include <ydb/core/io_formats/ydb_dump/csv_ydb_dump.h>
#include <ydb/public/lib/scheme_types/scheme_type_id.h>

Expand Down
18 changes: 18 additions & 0 deletions ydb/core/tx/schemeshard/ut_backup/ut_backup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,27 @@
#include <util/string/cast.h>
#include <util/string/printf.h>

#include <library/cpp/testing/hook/hook.h>

#include <aws/core/Aws.h>

using namespace NSchemeShardUT_Private;
using namespace NKikimr::NWrappers::NTestHelpers;

namespace {

Aws::SDKOptions Options;

Y_TEST_HOOK_BEFORE_RUN(InitAwsAPI) {
Aws::InitAPI(Options);
}

Y_TEST_HOOK_AFTER_RUN(ShutdownAwsAPI) {
Aws::ShutdownAPI(Options);
}

}

Y_UNIT_TEST_SUITE(TBackupTests) {
using TFillFn = std::function<void(TTestBasicRuntime&)>;

Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_backup/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ IF (NOT OS_WINDOWS)
library/cpp/getopt
library/cpp/regex/pcre
library/cpp/svnversion
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
ydb/core/testlib/default
ydb/core/tx
ydb/core/tx/schemeshard/ut_helpers
Expand Down
18 changes: 16 additions & 2 deletions ydb/core/tx/schemeshard/ut_export/ut_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,25 @@
#include <util/string/printf.h>
#include <util/system/env.h>

#include <library/cpp/testing/hook/hook.h>

#include <aws/core/Aws.h>

using namespace NSchemeShardUT_Private;
using namespace NKikimr::NWrappers::NTestHelpers;

namespace {

Aws::SDKOptions Options;

Y_TEST_HOOK_BEFORE_RUN(InitAwsAPI) {
Aws::InitAPI(Options);
}

Y_TEST_HOOK_AFTER_RUN(ShutdownAwsAPI) {
Aws::ShutdownAPI(Options);
}

void Run(TTestBasicRuntime& runtime, TTestEnv& env, const TVector<TString>& tables, const TString& request,
Ydb::StatusIds::StatusCode expectedStatus = Ydb::StatusIds::SUCCESS,
const TString& dbName = "/MyRoot", bool serverless = false, const TString& userSID = "", const TString& peerName = "") {
Expand Down Expand Up @@ -1687,7 +1701,7 @@ partitioning_settings {
return ev->Get<TEvSchemeShard::TEvModifySchemeTransaction>()->Record
.GetTransaction(0).GetOperationType() == NKikimrSchemeOp::ESchemeOpBackup;
};

THolder<IEventHandle> delayed;
auto prevObserver = runtime.SetObserverFunc([&](TAutoPtr<IEventHandle>& ev) {
if (delayFunc(ev)) {
Expand Down Expand Up @@ -2083,7 +2097,7 @@ partitioning_settings {
min_partitions_count: 10
)"));
}

Y_UNIT_TEST(UserSID) {
TTestBasicRuntime runtime;
TTestEnv env(runtime);
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_export/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ IF (NOT OS_WINDOWS)
library/cpp/getopt
library/cpp/regex/pcre
library/cpp/svnversion
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
ydb/core/testlib/default
ydb/core/tx
ydb/core/tx/schemeshard/ut_helpers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,28 @@

#include <util/string/printf.h>

#include <library/cpp/testing/hook/hook.h>

#include <aws/core/Aws.h>

using namespace NSchemeShardUT_Private;
using namespace NSchemeShardUT_Private::NExportReboots;
using namespace NKikimr::NWrappers::NTestHelpers;

namespace {

Aws::SDKOptions Options;

Y_TEST_HOOK_BEFORE_RUN(InitAwsAPI) {
Aws::InitAPI(Options);
}

Y_TEST_HOOK_AFTER_RUN(ShutdownAwsAPI) {
Aws::ShutdownAPI(Options);
}

}

Y_UNIT_TEST_SUITE(TExportToS3WithRebootsTests) {
using TUnderlying = std::function<void(const TVector<TString>&, const TString&, TTestWithReboots&)>;

Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_export_reboots_s3/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ PEERDIR(
library/cpp/getopt
library/cpp/regex/pcre
library/cpp/svnversion
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
ydb/core/testlib/default
ydb/core/tx
ydb/core/tx/schemeshard/ut_helpers
Expand Down
13 changes: 12 additions & 1 deletion ydb/core/tx/schemeshard/ut_restore/ut_restore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@

#include <ydb/public/api/protos/ydb_import.pb.h>

#include <aws/core/Aws.h>
#include <contrib/libs/zstd/include/zstd.h>
#include <library/cpp/string_utils/quote/quote.h>
#include <library/cpp/testing/hook/hook.h>

#include <util/datetime/base.h>
#include <util/generic/size_literals.h>
Expand All @@ -38,6 +40,16 @@ using namespace NKikimr::NWrappers::NTestHelpers;

namespace {

Aws::SDKOptions Options;

Y_TEST_HOOK_BEFORE_RUN(InitAwsAPI) {
Aws::InitAPI(Options);
}

Y_TEST_HOOK_AFTER_RUN(ShutdownAwsAPI) {
Aws::ShutdownAPI(Options);
}

const TString EmptyYsonStr = R"([[[[];%false]]])";

TString GenerateScheme(const NKikimrSchemeOp::TPathDescription& pathDesc) {
Expand Down Expand Up @@ -317,7 +329,6 @@ namespace {
runtime.SetObserverFunc(prevObserver);
}


} // anonymous

Y_UNIT_TEST_SUITE(TRestoreTests) {
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_restore/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ELSE()
ENDIF()

PEERDIR(
contrib/libs/aws-sdk-cpp/aws-cpp-sdk-core
contrib/libs/double-conversion
library/cpp/string_utils/quote
ydb/core/kqp/ut/common
Expand Down
3 changes: 1 addition & 2 deletions ydb/core/wrappers/s3_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#ifndef KIKIMR_DISABLE_S3_OPS

#include "abstract.h"
#include "s3_storage_config.h"

#include <ydb/core/protos/flat_scheme_op.pb.h>
#include <ydb/core/wrappers/events/common.h>
Expand Down Expand Up @@ -32,7 +31,7 @@

namespace NKikimr::NWrappers::NExternalStorage {

class TS3ExternalStorage: public IExternalStorageOperator, TS3User {
class TS3ExternalStorage: public IExternalStorageOperator {
private:
THolder<Aws::S3::S3Client> Client;
const Aws::Client::ClientConfiguration Config;
Expand Down
Loading

0 comments on commit f4c79be

Please sign in to comment.