Bug: Delete() cannot clean up containers with invalid PID values (Pid <= 0)

[Nachiket-Roy]

During container creation, unikontainer.InitialSetup() persists the container state with:

Pid = -1
Status = creating

If container creation fails before unikontainer.Create() updates the PID, the container remains in this intermediate state.

Later, when attempting to clean up the failed container using:

urunc delete <container-id>

Delete() invokes isRunning(), which currently performs:

syscall.Kill(u.State.Pid, syscall.Signal(0))

without validating the PID value.

When Pid == -1, the call becomes:

syscall.Kill(-1, 0)

On Unix systems, kill(-1, sig) targets all processes that the caller has permission to signal. When running as a privileged user, this call succeeds, causing isRunning() to incorrectly return true.

As a result, Delete() refuses to remove the container and reports:

cannot delete running container: <container-id>

This leaves failed containers stuck in the creating state and prevents cleanup of the associated runtime state.

Steps to Reproduce

1. Trigger a failure after unikontainer.InitialSetup() has completed but before unikontainer.Create() updates the PID.
2. Observe that the persisted state contains:

{
  "status": "creating",
  "pid": -1
}

3. Attempt to remove the failed container:

urunc delete <container-id>

Actual Behavior

Delete() reports that the container is still running:

cannot delete running container: <container-id>

and the runtime state cannot be cleaned up.

Expected Behavior

Containers with non-positive PIDs (Pid <= 0) should be considered not running, allowing cleanup to proceed.

Suggested Fix

Guard against invalid PID values in isRunning():

func (u *Unikontainer) isRunning() bool {
    if u.State.Pid <= 0 {
        return false
    }

    err := syscall.Kill(u.State.Pid, syscall.Signal(0))
    return err == nil
}

Additional Notes

InitialSetup() intentionally persists state before container creation completes, so failures during subsequent setup phases can legitimately leave containers with Pid = -1. Handling this case explicitly would allow these partially-created containers to be cleaned up correctly.

[cmainas]

Yes, this can cause issues if somehow the setup phase fails. A first iteration would be to add the suggested check for the pid, but we might need to rehting how InitialSetup handles the pid.

[priyamkarn]

Yes, this can cause issues if somehow the setup phase fails. A first iteration would be to add the suggested check for the pid, but we might need to rehting how InitialSetup handles the pid.

The isRunning() guard correctly fixes the reported issue, but there are a few other places that will hit the same -1 PID problem:

• Kill() calls vmm.Stop(u.State.Pid) with no guard — delete --force on a stuck container will pass -1 to the VMM stop logic
• Signal() calls vmm.Signal(u.State.Pid, ...) with the same issue
• Delete() builds /proc/%d/ns/net from u.State.Pid for network namespace cleanup — that path becomes /proc/-1/ns/net for a stuck container

So the PR fixes the plain delete path but delete --force and any signal path are still exposed.

For the tests, we only cover isRunning() directly — worth adding a test for delete --force on a Pid = -1 container since that's the next failure mode someone will hit.

Longer term, checking Status == creating in isRunning() (and the other guards) is probably cleaner than a sentinel PID check, since the status field already models this lifecycle stage explicitly. But the Pid <= 0 guard is the right thing to merge now as a safe, non-breaking first step.