Skip to content

Commit

Permalink
Restrict stopping actors (#3878)
Browse files Browse the repository at this point in the history
* Only allow stopping by parent actorContext or by stopping the system

* Fix types + tests

* Add changeset

* Bring back old stop method but forbid non-root stops

* Add noop `stop` to `toActorRef`

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
davidkpiano and Andarist committed Mar 30, 2023
1 parent c4e58c8 commit bb91037
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/lemon-pans-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': major
---

Actors can no longer be stopped directly by calling ~~`actor.stop()`~~. They can only be stopped from its parent internally (which might happen when you use `stop` action or automatically when a machine leaves the invoking state). The root actor can still be stopped since it has no parent.
4 changes: 2 additions & 2 deletions packages/core/src/actions/stop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ export function stop<
return;
}
if (actorRef.status !== ActorStatus.Running) {
actorRef.stop?.();
actorCtx.stopChild(actorRef);
return;
}
actorCtx.defer(() => {
actorRef.stop?.();
actorCtx.stopChild(actorRef);
});
}
} as StopActionObject
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/actors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function toActorRef<
return this;
},
status: ActorStatus.Running,
stop: () => void 0,
...actorRefLike
};
}
31 changes: 23 additions & 8 deletions packages/core/src/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,15 @@ export class Interpreter<
defer: (fn) => {
this._deferred.push(fn);
},
system: this.system
system: this.system,
stopChild: (child) => {
if (child._parent !== this) {
throw new Error(
`Cannot stop child actor ${child.id} of ${this.id} because it is not a child`
);
}
(child as any)._stop();
}
};

// Ensure that the send method is bound to this interpreter instance
Expand Down Expand Up @@ -212,7 +220,7 @@ export class Interpreter<
})
);

this._stop();
this._stopProcedure();
break;
case 'error':
this._parent?.send(
Expand Down Expand Up @@ -324,7 +332,7 @@ export class Interpreter<
this.update(nextState);

if (event.name === stopSignalType) {
this._stop();
this._stopProcedure();
}
} catch (err) {
// TODO: properly handle errors
Expand All @@ -339,10 +347,7 @@ export class Interpreter<
}
}

/**
* Stops the interpreter and unsubscribe all listeners.
*/
public stop(): this {
private _stop(): this {
if (this.status === ActorStatus.Stopped) {
return this;
}
Expand All @@ -355,13 +360,23 @@ export class Interpreter<

return this;
}

/**
* Stops the interpreter and unsubscribe all listeners.
*/
public stop(): this {
if (this._parent) {
throw new Error('A non-root actor cannot be stopped directly.');
}
return this._stop();
}
private _complete(): void {
for (const observer of this.observers) {
observer.complete?.();
}
this.observers.clear();
}
private _stop(): this {
private _stopProcedure(): this {
this._complete();

if (this.status !== ActorStatus.Running) {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,7 @@ export interface ActorRef<TEvent extends EventObject, TSnapshot = any>
getSnapshot: () => TSnapshot | undefined;
// TODO: this should return some sort of TPersistedState, not any
getPersistedState?: () => any;
stop?: () => void;
stop: () => void;
toJSON?: () => any;
// TODO: figure out how to hide this externally as `sendTo(ctx => ctx.actorRef._parent._parent._parent._parent)` shouldn't be allowed
_parent?: ActorRef<any, any>;
Expand Down Expand Up @@ -1867,6 +1867,7 @@ export interface ActorContext<
logger: (...args: any[]) => void;
defer: (fn: () => void) => void;
system: TSystem;
stopChild: (child: AnyActorRef) => void;
}

export type AnyActorContext = ActorContext<any, any, any>;
Expand Down

0 comments on commit bb91037

Please sign in to comment.