Skip to content

24.3: Fix null dereference in node broker (KIKIMR-21693) (#6516) #6623

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 12 additions & 6 deletions ydb/core/mind/node_broker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand All @@ -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,
Expand Down Expand Up @@ -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)
{
Expand Down
15 changes: 6 additions & 9 deletions ydb/core/mind/node_broker__register_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
24 changes: 21 additions & 3 deletions ydb/core/mind/node_broker_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ydb/public/lib/ydb_cli/commands/ydb_tools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
Expand Down
Loading