Automatically lock state on invoke (#1351)

* Automatically lock state on invoke

* Additional locking for admin commissioning

* Do not autolock admin commissioning on invoke

* Do not lock general commissioning on invoke

CHANGELOG.md

@@ -12,6 +12,7 @@ The main work (all changes without a GitHub username in brackets in the below li
 ## __WORK IN PROGRESS__
 
 -   @matter/node
+    - Feature: Automatically lock behavior state on invoke
     - Fix: Ensures to fully load the Descriptor cluster before adding additional device types 
 
 -   @project-chip/matter.js

packages/node/src/behavior/Behavior.ts

@@ -6,6 +6,7 @@
 
 import { Agent, INSTALL_BEHAVIOR } from "#endpoint/Agent.js";
 import {
+    AsyncObservable,
     EventEmitter,
     GeneratedClass,
     ImplementationError,
@@ -239,6 +240,19 @@ export abstract class Behavior {
         return (...args: A) => observable.emit(...args);
     }
 
+    /**
+     * Create an async callback.
+     *
+     * @see {@link callback}
+     */
+    protected asyncCallback<A extends any[], R>(reactor: Reactor<A, R>, options?: Reactor.Options) {
+        const observable = AsyncObservable<A, R>();
+
+        this.reactTo(observable, reactor, options);
+
+        return (...args: A) => observable.emit(...args);
+    }
+
     /**
      * Does this behavior support functionality of a specific implementation?
      */

packages/node/src/behavior/cluster/ClusterBehavior.ts

@@ -72,6 +72,11 @@ export class ClusterBehavior extends Behavior {
      */
     static override readonly dependencies = [NetworkBehavior];
 
+    /**
+     * Automatically lock state on command invoke.
+     */
+    static readonly lockOnInvoke = true;
+
     constructor(agent: Agent, backing: BehaviorBacking) {
         super(agent, backing);
 
@@ -221,6 +226,7 @@ export namespace ClusterBehavior {
         readonly dependencies?: Iterable<Behavior.Type>;
         supports: typeof ClusterBehavior.supports;
         readonly ExtensionInterface: ExtensionInterfaceOf<B>;
+        readonly lockOnInvoke: boolean;
 
         // Prior to TS 5.4 could do this.  Sadly typing no longer carries through on these...  This["cluster"] reverts
         // to ClusterType).  So we have to define the long way.

packages/node/src/behavior/internal/ClusterServerBacking.ts

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { Resource } from "#behavior/state/index.js";
 import type { EndpointServer } from "#endpoint/EndpointServer.js";
 import { camelize, Diagnostic, ImplementationError, InternalError, isObject, Logger, MaybePromise } from "#general";
 import { CommandModel, ElementTag } from "#model";
@@ -332,7 +333,28 @@ function createCommandServer(name: string, definition: Command<any, any, any>, b
         try {
             activity = behavior.context?.activity?.frame(`invoke ${name}`);
 
-            result = (behavior as unknown as Record<string, (arg: unknown) => unknown>)[name](request);
+            const invoke = (behavior as unknown as Record<string, (arg: unknown) => unknown>)[name].bind(behavior);
+
+            // Lock if necessary, then invoke
+            if ((behavior.constructor as ClusterBehavior.Type).lockOnInvoke) {
+                const tx = behavior.context.transaction;
+                if (Resource.isLocked(behavior)) {
+                    // Automatic locking with locked resource; requires async lock acquisition
+                    result = (async function invokeAsync() {
+                        await tx.addResources(behavior);
+                        await tx.begin();
+                        return invoke(request);
+                    })();
+                } else {
+                    // Automatic locking on unlocked resource; may proceed synchronously
+                    tx.addResourcesSync(behavior);
+                    tx.beginSync();
+                    result = invoke(request);
+                }
+            } else {
+                // Automatic locking disabled
+                result = invoke(request);
+            }
 
             if (MaybePromise.is(result)) {
                 isAsync = true;

packages/node/src/behavior/state/transaction/Resource.ts

@@ -29,4 +29,11 @@ export interface Resource {
 
 export namespace Resource {
     export const reference = Symbol("reference");
+
+    export function isLocked(resource: Resource) {
+        while (resource[Resource.reference] !== undefined) {
+            resource = resource[Resource.reference];
+        }
+        return resource.lockedBy !== undefined;
+    }
 }

packages/node/src/behaviors/administrator-commissioning/AdministratorCommissioningServer.ts

@@ -61,6 +61,8 @@ export class AdministratorCommissioningServer extends AdministratorCommissioning
     declare internal: AdministratorCommissioningServer.Internal;
     declare state: AdministratorCommissioningServer.State;
 
+    static override lockOnInvoke = false;
+
     /**
      * This method opens an Enhanced Commissioning Window (a dynamic passcode is used which was provided by the caller).
      */
@@ -244,7 +246,6 @@ export class AdministratorCommissioningServer extends AdministratorCommissioning
      * Closes the commissioning window per the matter specification.
      */
     async #closeCommissioningWindow() {
-        this.callback(this.#endCommissioning);
         await this.env.get(DeviceCommissioner).endCommissioning();
     }
 

packages/node/src/behaviors/general-commissioning/GeneralCommissioningServer.ts

@@ -23,6 +23,8 @@ const logger = Logger.get("GeneralCommissioningClusterHandler");
 export class GeneralCommissioningServer extends GeneralCommissioningBehavior {
     declare state: GeneralCommissioningServer.State;
 
+    static override lockOnInvoke = false;
+
     override initialize() {
         const bci = this.state.basicCommissioningInfo;
 

packages/protocol/src/protocol/DeviceCommissioner.ts

@@ -17,6 +17,7 @@ import {
     Lifecycle,
     Logger,
     MatterFlowError,
+    MaybePromise,
     ObserverGroup,
 } from "#general";
 import { SecureChannelProtocol } from "#securechannel/SecureChannelProtocol.js";
@@ -56,7 +57,7 @@ export class DeviceCommissioner {
     #failsafeContext?: FailsafeContext;
     #windowStatus = AdministratorCommissioning.CommissioningWindowStatus.WindowNotOpen;
     #activeDiscriminator?: number;
-    #activeCommissioningEndCallback?: () => void;
+    #activeCommissioningEndCallback?: () => MaybePromise;
     #observers = new ObserverGroup(this);
 
     constructor(context: DeviceCommissionerContext) {
@@ -117,7 +118,7 @@ export class DeviceCommissioner {
     async allowEnhancedCommissioning(
         discriminator: number,
         paseServer: PaseServer,
-        commissioningEndCallback: () => void,
+        commissioningEndCallback: () => MaybePromise,
     ) {
         if (this.#windowStatus === AdministratorCommissioning.CommissioningWindowStatus.BasicWindowOpen) {
             throw new MatterFlowError(
@@ -133,7 +134,7 @@ export class DeviceCommissioner {
         );
     }
 
-    async allowBasicCommissioning(commissioningEndCallback?: () => void) {
+    async allowBasicCommissioning(commissioningEndCallback?: () => MaybePromise) {
         if (this.#windowStatus === AdministratorCommissioning.CommissioningWindowStatus.EnhancedWindowOpen) {
             throw new MatterFlowError(
                 "Enhanced commissioning window is already open! Cannot set Basic commissioning mode.",
@@ -217,7 +218,7 @@ export class DeviceCommissioner {
 
     async #becomeCommissionable(
         windowStatus: AdministratorCommissioning.CommissioningWindowStatus,
-        activeCommissioningEndCallback?: () => void,
+        activeCommissioningEndCallback?: () => MaybePromise,
         discriminator?: number,
     ) {
         if (
@@ -261,7 +262,7 @@ export class DeviceCommissioner {
         if (this.#activeCommissioningEndCallback !== undefined) {
             const activeCommissioningEndCallback = this.#activeCommissioningEndCallback;
             this.#activeCommissioningEndCallback = undefined;
-            activeCommissioningEndCallback();
+            await activeCommissioningEndCallback();
         }
 
         await this.#context.advertiser.exitCommissioningMode();