Skip to content

Fix use-after-dtor in TTabletMetrics #6873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
efimov-ivan-yteam wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −3
Open

Fix use-after-dtor in TTabletMetrics #6873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions cloud/filestore/libs/storage/tablet/tablet_counters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ TTabletMetrics::TTabletMetrics(IMetricsRegistryPtr metricsRegistry)
, AggregatableFsRegistry(CreateMetricsRegistryStub())
{}

TTabletMetrics::~TTabletMetrics()
{
MaxUsedQuota.Unregister(MaxUsedQuotaKey);
ReadDataPostponed.Unregister(ReadDataPostponedKey);
WriteDataPostponed.Unregister(WriteDataPostponedKey);

AggregatableFsRegistry.reset();
FsRegistry.reset();

@robot-vibe-db robot-vibe-db Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: Minor
Confidence: Low

When TTabletMetrics::Register() was never called (Initialized == false), the three keys are default-constructed TMetricKey(0, 0). The Unregister() calls become harmless no-ops (Registries.erase() on non-existent keys), and the reset() calls release the initial stubs — so this is safe.

However, adding an early-return guard would make the intent explicit and skip three unnecessary lock acquisitions:

TTabletMetrics::~TTabletMetrics()
{
    if (!Initialized) {
        return;
    }

    MaxUsedQuota.Unregister(MaxUsedQuotaKey);
    ...
}

}

////////////////////////////////////////////////////////////////////////////////

void TTabletMetrics::Register(
Expand Down Expand Up @@ -334,14 +344,14 @@ void TTabletMetrics::Register(
REGISTER_LOCAL(RejectedRequests, EMetricType::MT_DERIVATIVE);
REGISTER_LOCAL(PostponedRequests, EMetricType::MT_DERIVATIVE);
REGISTER_LOCAL(UsedQuota, EMetricType::MT_DERIVATIVE);
MaxUsedQuota.Register(
MaxUsedQuotaKey = MaxUsedQuota.Register(
FsRegistry,
{CreateSensor("MaxUsedQuota")},
EAggregationType::AT_MAX);
ReadDataPostponed.Register(
ReadDataPostponedKey = ReadDataPostponed.Register(
FsRegistry,
{CreateLabel("request", "ReadData"), CreateLabel("histogram", "ThrottlerDelay")});
WriteDataPostponed.Register(
WriteDataPostponedKey = WriteDataPostponed.Register(
FsRegistry,
{CreateLabel("request", "WriteData"), CreateLabel("histogram", "ThrottlerDelay")});

Expand Down
7 changes: 7 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_counters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "tablet_tx.h"

#include <cloud/filestore/libs/diagnostics/metrics/histogram.h>
#include <cloud/filestore/libs/diagnostics/metrics/key.h>
#include <cloud/filestore/libs/diagnostics/metrics/public.h>
#include <cloud/filestore/libs/diagnostics/metrics/window_calculator.h>

Expand Down Expand Up @@ -332,7 +333,13 @@ struct TTabletMetrics: TAtomicRefCount<TTabletMetrics>
NMetrics::IMetricsRegistryPtr FsRegistry;
NMetrics::IMetricsRegistryPtr AggregatableFsRegistry;

// Keys of the registrations that hold FsRegistry alive, needed by ~TTabletMetrics().
NMetrics::TMetricKey MaxUsedQuotaKey;
NMetrics::TMetricKey ReadDataPostponedKey;
NMetrics::TMetricKey WriteDataPostponedKey;

explicit TTabletMetrics(NMetrics::IMetricsRegistryPtr metricsRegistry);
~TTabletMetrics();

void Register(
const TString& fsId,
Expand Down
Loading