services updates

docs/services.md

@@ -84,6 +84,17 @@ the network always reflects the current code's configuration instead of silently
 whatever options a previous node version created it with, and it matches restart's overall
 tear-down-and-rebuild semantics (the container is never reused either).
 
+**Lifecycle operations are exclusive per service.** At most one lifecycle operation — the
+background start pipeline, `SERVICE_RESTART`, `SERVICE_STOP`, or the expiry sweep — runs per
+service at a time. A restart or stop issued while another operation is in flight (e.g. a
+restart still pulling the image) is rejected with
+`Service <id> has a start/stop/restart operation in progress — retry shortly`; simply retry
+once the in-flight operation settles. Without this exclusivity, the background loop's
+crash-orphan recovery could tear down the `ocean-svc-<serviceId>` network in the middle of a
+restart that had just created it, failing the restart with
+`network ocean-svc-<id> not found`. If a service expires while such an operation is in
+flight, the expiry sweep simply retries on a later tick.
+
 ## Configuration
 
 Service-on-demand is configured per Docker connection under `serviceOnDemand`:

src/components/c2d/compute_engine_docker.ts

@@ -104,9 +104,14 @@ export class C2DEngineDocker extends C2DEngine {
   private stopped: boolean = false
   // The currently-running InternalLoop pass, so stop() can drain it before returning.
   private internalLoopPromise: Promise<void> | null = null
-  // serviceIds currently being advanced by processServiceStart, so the InternalLoop doesn't
-  // launch a second pipeline for the same service while one is already in flight.
-  private servicesBeingStarted: Set<string> = new Set()
+  // Per-service lifecycle lock: serviceIds with an exclusive operation in flight — the
+  // loop-driven start pipeline (processServiceStart), restartService or stopService. At
+  // most one such operation may run per service: the InternalLoop skips locked ids
+  // (including its expiry sweep), restart/stop throw. Without this, the loop's
+  // orphan-recovery tears down the network a concurrent restart just created (the
+  // "network ocean-svc-<id> not found" failure) and start/stop/restart clobber each
+  // other's docker resources and job status.
+  private serviceOpsInFlight: Set<string> = new Set()
   // The in-flight processServiceStart() promises (launched fire-and-forget by InternalLoop),
   // so stop() can drain them before returning — otherwise a start could outlive stop() and
   // race a restarted engine on the same shared DB.
@@ -1787,11 +1792,11 @@ export class C2DEngineDocker extends C2DEngine {
         this.getC2DConfig().hash
       )
       for (const svc of pendingStarts) {
-        if (this.servicesBeingStarted.has(svc.serviceId)) continue
-        this.servicesBeingStarted.add(svc.serviceId)
+        if (this.serviceOpsInFlight.has(svc.serviceId)) continue
+        this.serviceOpsInFlight.add(svc.serviceId)
         // Track the promise so stop() can drain it; clean both trackers when it settles.
         const startPromise = this.processServiceStart(svc).finally(() => {
-          this.servicesBeingStarted.delete(svc.serviceId)
+          this.serviceOpsInFlight.delete(svc.serviceId)
           this.serviceStartPromises.delete(startPromise)
         })
         this.serviceStartPromises.add(startPromise)
@@ -1803,11 +1808,17 @@ export class C2DEngineDocker extends C2DEngine {
       )
       for (const svc of expiredServices) {
         CORE_LOGGER.info(`Service ${svc.serviceId} expired — stopping`)
-        await this.stopService(svc.serviceId, svc.owner).catch((e) => {
+        try {
+          await this.stopService(svc.serviceId, svc.owner)
+        } catch (e: any) {
+          // Typically the lifecycle lock (a restart/stop already in flight). Marking
+          // Expired without teardown would leak the container/ports, so defer the whole
+          // sweep for this service to the next tick.
           CORE_LOGGER.error(
-            `Failed to stop expired service ${svc.serviceId}: ${e.message}`
+            `Failed to stop expired service ${svc.serviceId}: ${e.message} — retrying next tick`
           )
-        })
+          continue
+        }
         // mark the (now stopped) record as Expired so it is not picked up again
         const [stoppedJob] = await this.db.getServiceJob(svc.serviceId, svc.owner)
         if (stoppedJob) {
@@ -3671,9 +3682,35 @@ export class C2DEngineDocker extends C2DEngine {
     CORE_LOGGER.error(`Service ${job.serviceId} container died — ${reason}`)
   }
 
+  // Takes the per-service lifecycle lock or throws: a stop must not run while the loop's
+  // start pipeline or a restart owns the service — its by-name network removal would tear
+  // down the docker resources the other operation is creating (and vice versa).
+  private acquireServiceLifecycleLock(serviceId: string): void {
+    if (this.serviceOpsInFlight.has(serviceId)) {
+      throw new Error(
+        `Service ${serviceId} has a start/stop/restart operation in progress — retry shortly`
+      )
+    }
+    this.serviceOpsInFlight.add(serviceId)
+  }
+
   public override async stopService(
     serviceId: string,
     owner: string
+  ): Promise<ServiceJob | null> {
+    this.acquireServiceLifecycleLock(serviceId)
+    try {
+      return await this.doStopService(serviceId, owner)
+    } finally {
+      this.serviceOpsInFlight.delete(serviceId)
+    }
+  }
+
+  // The actual stop. Must only run while holding the service lifecycle lock (see
+  // stopService above).
+  private async doStopService(
+    serviceId: string,
+    owner: string
   ): Promise<ServiceJob | null> {
     const [job] = await this.db.getServiceJob(serviceId, owner)
     if (!job) return null
@@ -3728,6 +3765,33 @@ export class C2DEngineDocker extends C2DEngine {
     newUserData?: string,
     newDockerCmd?: string[],
     newDockerEntrypoint?: string[]
+  ): Promise<ServiceJob | null> {
+    // Lifecycle lock: without it the InternalLoop's orphan-recovery (which sees the
+    // intermediate PullImage/BuildImage status this method persists) tears down the
+    // network created here mid-restart → container.start() fails with
+    // "network ocean-svc-<id> not found".
+    this.acquireServiceLifecycleLock(serviceId)
+    try {
+      return await this.doRestartService(
+        serviceId,
+        owner,
+        newUserData,
+        newDockerCmd,
+        newDockerEntrypoint
+      )
+    } finally {
+      this.serviceOpsInFlight.delete(serviceId)
+    }
+  }
+
+  // The actual restart. Must only run while holding the service lifecycle lock (see
+  // restartService above).
+  private async doRestartService(
+    serviceId: string,
+    owner: string,
+    newUserData?: string,
+    newDockerCmd?: string[],
+    newDockerEntrypoint?: string[]
   ): Promise<ServiceJob | null> {
     const [job] = await this.db.getServiceJob(serviceId, owner)
     if (!job) return null

src/test/integration/services.test.ts

@@ -782,6 +782,90 @@ describe('**********         Service on Demand', () => {
     )
   })
 
+  it('(l3) SERVICE_RESTART survives InternalLoop ticks mid-restart (orphan-recovery race)', async function () {
+    this.timeout(DEFAULT_TEST_TIMEOUT * 4)
+    const engine: any = getDockerEngine()
+    const before = await getServiceJob(serviceId)
+    const oldContainerId = before.containerId
+
+    // Delay the image pull so the job sits in PullImage across several InternalLoop
+    // ticks (cronTime = 2 s) — the exact window in which the loop's orphan-recovery
+    // used to tear down the network the restart had just created, failing the restart
+    // with "network ocean-svc-<id> not found" (the live-node bug).
+    const originalPull = engine.pullImageRef
+    engine.pullImageRef = async (...args: any[]) => {
+      await sleep(5000)
+      return originalPull.apply(engine, args)
+    }
+    try {
+      const {
+        consumerAddress: addr,
+        nonce,
+        signature
+      } = await signFor(consumerAccount, PROTOCOL_COMMANDS.SERVICE_RESTART)
+      const task: ServiceRestartCommand = {
+        command: PROTOCOL_COMMANDS.SERVICE_RESTART,
+        consumerAddress: addr,
+        nonce,
+        signature,
+        serviceId
+      }
+      const restartPromise = new ServiceRestartHandler(oceanNode).handle(task)
+
+      // wait until the restart is actually mid-pull (deterministic, not sleep-based)
+      const deadline = Date.now() + 10_000
+      while (Date.now() < deadline) {
+        const j = await getServiceJob(serviceId)
+        if (j?.status === ServiceStatusNumber.PullImage) break
+        await sleep(250)
+      }
+
+      // concurrent lifecycle operations must be rejected while the restart holds the lock
+      const stopSig = await signFor(consumerAccount, PROTOCOL_COMMANDS.SERVICE_STOP)
+      const stopResp = await new ServiceStopHandler(oceanNode).handle({
+        command: PROTOCOL_COMMANDS.SERVICE_STOP,
+        consumerAddress: stopSig.consumerAddress,
+        nonce: stopSig.nonce,
+        signature: stopSig.signature,
+        serviceId
+      } as ServiceStopCommand)
+      expect(stopResp.status.httpStatus).to.not.equal(200)
+      expect(String(stopResp.status.error)).to.contain('operation in progress')
+
+      const retrySig = await signFor(consumerAccount, PROTOCOL_COMMANDS.SERVICE_RESTART)
+      const retryResp = await new ServiceRestartHandler(oceanNode).handle({
+        ...task,
+        nonce: retrySig.nonce,
+        signature: retrySig.signature
+      })
+      expect(retryResp.status.httpStatus).to.not.equal(200)
+      expect(String(retryResp.status.error)).to.contain('operation in progress')
+
+      // the original restart must complete unharmed by the loop ticks that fired mid-pull
+      const resp = await restartPromise
+      assert(
+        resp.status.httpStatus === 200,
+        `expected 200, got ${resp.status.httpStatus}: ${resp.status?.error ?? ''}`
+      )
+    } finally {
+      engine.pullImageRef = originalPull
+    }
+
+    // pollServiceStatus throws if the job lands in Error — which is exactly what the
+    // pre-fix orphan-recovery race produced ("Service start aborted (node restarted
+    // mid-start)" / "network ocean-svc-<id> not found").
+    const running = await pollServiceStatus(serviceId, ServiceStatusNumber.Running)
+    expect(running.containerId).to.not.equal(oldContainerId)
+    expect(running.endpoints[0].hostPort).to.equal(hostPort)
+    expect(running.expiresAt).to.equal(expiresAt)
+
+    const res = await httpGetWithRetry(endpointUrl)
+    assert(
+      res.status === 200,
+      `expected nginx HTTP 200 after raced restart, got ${res.status}`
+    )
+  })
+
   it('(m) SERVICE_STOP → Stopped, container + network removed', async function () {
     this.timeout(DEFAULT_TEST_TIMEOUT * 2)
     const before = await getServiceJob(serviceId)

src/test/unit/compute.test.ts

@@ -1512,9 +1512,11 @@ describe('service start/restart Docker cleanup on failure', function () {
     // Bypass the Docker-specific constructor but keep the prototype methods.
     engine = Object.create(C2DEngineDocker.prototype)
     // Constructor field initializers don't run via Object.create, so seed the CPU
-    // pinning maps that releaseCpus/allocateCpus (called by cleanupServiceDocker) touch.
+    // pinning maps that releaseCpus/allocateCpus (called by cleanupServiceDocker) touch,
+    // and the per-service lifecycle lock taken by stopService/restartService.
     engine.cpuAllocations = new Map()
     engine.envCpuCoresMap = new Map()
+    engine.serviceOpsInFlight = new Set()
     // inspect is needed by removeServiceNetwork (cleanup resolves the network by
     // deterministic name/id and checks for attached containers before removing).
     network = {

src/test/unit/service/serviceNetworkCleanup.test.ts

@@ -72,6 +72,7 @@ function makeEngine(): any {
     createNetwork: sinon.stub()
   }
   engine.cpuAllocations = new Map()
+  engine.serviceOpsInFlight = new Set()
   return engine
 }