KIKIMR-21693: Fix null dereference in node broker (ydb-platform#6516)

pixcc · Jul 12, 2024 · 90fbbe2 · 90fbbe2
1 parent 0e4b669
commit 90fbbe2
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 19 deletions.
diff --git a/ydb/core/mind/node_broker.cpp b/ydb/core/mind/node_broker.cpp
@@ -860,9 +860,9 @@ void TNodeBroker::Handle(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
     LOG_TRACE_S(ctx, NKikimrServices::NODE_BROKER, "Handle TEvNodeBroker::TEvRegistrationRequest"
         << ": request# " << ev->Get()->Record.ShortDebugString());
 
-    class TRegisterNodeActor : public TActorBootstrapped<TRegisterNodeActor> {
+    class TResolveTenantActor : public TActorBootstrapped<TResolveTenantActor> {
         TEvNodeBroker::TEvRegistrationRequest::TPtr Ev;
-        TNodeBroker *Self;
+        TActorId ReplyTo;
         NActors::TScopeId ScopeId;
         TSubDomainKey ServicedSubDomain;
 
@@ -871,9 +871,9 @@ void TNodeBroker::Handle(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
             return NKikimrServices::TActivity::NODE_BROKER_ACTOR;
         }
 
-        TRegisterNodeActor(TEvNodeBroker::TEvRegistrationRequest::TPtr& ev, TNodeBroker *self)
+        TResolveTenantActor(TEvNodeBroker::TEvRegistrationRequest::TPtr& ev, TActorId replyTo)
             : Ev(ev)
-            , Self(self)
+            , ReplyTo(replyTo)
         {}
 
         void Bootstrap(const TActorContext& ctx) {
@@ -930,7 +930,7 @@ void TNodeBroker::Handle(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
                 << ": scope id# " << ScopeIdToString(ScopeId)
                 << ": serviced subdomain# " << ServicedSubDomain);
 
-            Self->ProcessTx(Self->CreateTxRegisterNode(Ev, ScopeId, ServicedSubDomain), ctx);
+            Send(ReplyTo, new TEvPrivate::TEvResolvedRegistrationRequest(Ev, ScopeId, ServicedSubDomain));
             Die(ctx);
         }
 
@@ -939,7 +939,7 @@ void TNodeBroker::Handle(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
             CFunc(TEvents::TSystem::Undelivered, HandleUndelivered)
         })
     };
-    ctx.RegisterWithSameMailbox(new TRegisterNodeActor(ev, this));
+    ctx.RegisterWithSameMailbox(new TResolveTenantActor(ev, SelfId()));
 }
 
 void TNodeBroker::Handle(TEvNodeBroker::TEvExtendLeaseRequest::TPtr &ev,
@@ -989,6 +989,12 @@ void TNodeBroker::Handle(TEvPrivate::TEvUpdateEpoch::TPtr &ev,
     ProcessTx(CreateTxUpdateEpoch(), ctx);
 }
 
+void TNodeBroker::Handle(TEvPrivate::TEvResolvedRegistrationRequest::TPtr &ev,
+                         const TActorContext &ctx)
+{
+    ProcessTx(CreateTxRegisterNode(ev), ctx);
+}
+
 IActor *CreateNodeBroker(const TActorId &tablet,
                          TTabletStorageInfo *info)
 {

diff --git a/ydb/core/mind/node_broker__register_node.cpp b/ydb/core/mind/node_broker__register_node.cpp
@@ -10,12 +10,11 @@ using namespace NKikimrNodeBroker;
 
 class TNodeBroker::TTxRegisterNode : public TTransactionBase<TNodeBroker> {
 public:
-    TTxRegisterNode(TNodeBroker *self, TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
-                    const NActors::TScopeId& scopeId, const TSubDomainKey& servicedSubDomain)
+    TTxRegisterNode(TNodeBroker *self, TEvPrivate::TEvResolvedRegistrationRequest::TPtr &resolvedEv)
         : TBase(self)
-        , Event(ev)
-        , ScopeId(scopeId)
-        , ServicedSubDomain(servicedSubDomain)
+        , Event(resolvedEv->Get()->Request)
+        , ScopeId(resolvedEv->Get()->ScopeId)
+        , ServicedSubDomain(resolvedEv->Get()->ServicedSubDomain)
         , NodeId(0)
         , ExtendLease(false)
         , FixNodeId(false)
@@ -186,11 +185,9 @@ class TNodeBroker::TTxRegisterNode : public TTransactionBase<TNodeBroker> {
     bool FixNodeId;
 };
 
-ITransaction *TNodeBroker::CreateTxRegisterNode(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
-                                                const NActors::TScopeId& scopeId,
-                                                const TSubDomainKey& servicedSubDomain)
+ITransaction *TNodeBroker::CreateTxRegisterNode(TEvPrivate::TEvResolvedRegistrationRequest::TPtr &ev)
 {
-    return new TTxRegisterNode(this, ev, scopeId, servicedSubDomain);
+    return new TTxRegisterNode(this, ev);
 }
 
 } // NNodeBroker

diff --git a/ydb/core/mind/node_broker_impl.h b/ydb/core/mind/node_broker_impl.h
@@ -46,13 +46,30 @@ class TNodeBroker : public TActor<TNodeBroker>
     struct TEvPrivate {
         enum EEv {
             EvUpdateEpoch = EventSpaceBegin(TEvents::ES_PRIVATE),
+            EvResolvedRegistrationRequest,
 
             EvEnd
         };
 
         static_assert(EvEnd < EventSpaceEnd(TKikimrEvents::ES_PRIVATE), "expect EvEnd < EventSpaceEnd(TKikimrEvents::ES_PRIVATE)");
 
         struct TEvUpdateEpoch : public TEventLocal<TEvUpdateEpoch, EvUpdateEpoch> {};
+
+        struct TEvResolvedRegistrationRequest : public TEventLocal<TEvResolvedRegistrationRequest, EvResolvedRegistrationRequest> {
+
+            TEvResolvedRegistrationRequest(
+                    TEvNodeBroker::TEvRegistrationRequest::TPtr request,
+                    NActors::TScopeId scopeId,
+                    TSubDomainKey servicedSubDomain)
+                : Request(request)
+                , ScopeId(scopeId)
+                , ServicedSubDomain(servicedSubDomain)
+            {}
+
+            TEvNodeBroker::TEvRegistrationRequest::TPtr Request;
+            NActors::TScopeId ScopeId;
+            TSubDomainKey ServicedSubDomain;
+        };
     };
 
 private:
@@ -138,9 +155,7 @@ class TNodeBroker : public TActor<TNodeBroker>
     ITransaction *CreateTxExtendLease(TEvNodeBroker::TEvExtendLeaseRequest::TPtr &ev);
     ITransaction *CreateTxInitScheme();
     ITransaction *CreateTxLoadState();
-    ITransaction *CreateTxRegisterNode(TEvNodeBroker::TEvRegistrationRequest::TPtr &ev,
-                                       const NActors::TScopeId& scopeId,
-                                       const TSubDomainKey& servicedSubDomain);
+    ITransaction *CreateTxRegisterNode(TEvPrivate::TEvResolvedRegistrationRequest::TPtr &ev);
     ITransaction *CreateTxUpdateConfig(TEvConsole::TEvConfigNotificationRequest::TPtr &ev);
     ITransaction *CreateTxUpdateConfig(TEvNodeBroker::TEvSetConfigRequest::TPtr &ev);
     ITransaction *CreateTxUpdateConfigSubscription(TEvConsole::TEvReplaceConfigSubscriptionsResponse::TPtr &ev);
@@ -192,6 +207,7 @@ class TNodeBroker : public TActor<TNodeBroker>
             HFuncTraced(TEvNodeBroker::TEvGetConfigRequest, Handle);
             HFuncTraced(TEvNodeBroker::TEvSetConfigRequest, Handle);
             HFuncTraced(TEvPrivate::TEvUpdateEpoch, Handle);
+            HFuncTraced(TEvPrivate::TEvResolvedRegistrationRequest, Handle);
             IgnoreFunc(TEvTabletPipe::TEvServerConnected);
             IgnoreFunc(TEvTabletPipe::TEvServerDisconnected);
             IgnoreFunc(NConsole::TEvConfigsDispatcher::TEvSetConfigSubscriptionResponse);
@@ -293,6 +309,8 @@ class TNodeBroker : public TActor<TNodeBroker>
                 const TActorContext &ctx);
     void Handle(TEvPrivate::TEvUpdateEpoch::TPtr &ev,
                 const TActorContext &ctx);
+    void Handle(TEvPrivate::TEvResolvedRegistrationRequest::TPtr &ev,
+                const TActorContext &ctx);
 
     // All registered dynamic nodes.
     THashMap<ui32, TNodeInfo> Nodes;

diff --git a/ydb/public/lib/ydb_cli/commands/ydb_tools.cpp b/ydb/public/lib/ydb_cli/commands/ydb_tools.cpp
@@ -295,7 +295,7 @@ int TCommandCopy::Run(TConfig& config) {
 ////////////////////////////////////////////////////////////////////////////////
 
 TCommandRename::TCommandRename()
-    : TTableCommand("rename", {}, "Rename or repalce table(s)")
+    : TTableCommand("rename", {}, "Rename or replace table(s)")
 {
     TItem::DefineFields({
         {"Source", {{"source", "src"}, "Source table path", true}},