fix(servstate): don't hold both servicesLock and state lock at once (#359)
* fix(servstate): don't hold both servicesLock and state lock at once
This avoids the 3-lock deadlock described in https://github.com/canonical/pebble/issues/314. Other goroutines may be
holding the state lock and waiting for the services lock, so it's
problematic to acquire both locks at once. Break that part of the
cycle.
We could do this inside serviceForStart/serviceForStop by manually
calling Unlock() sooner, but that's error-prone, so continue using
defer, but have the caller write the task log (which needs the state
lock) after the services lock is released.
fix(servstate): reduce scope of holding ServiceManager.planLock (#355)
* Don't lock unnecessarily around userFromRequest
There's also a bug here if userFromRequest starts returning an err:
we're not unlocking in the error case.
userFromRequest does nothing right now, so it's a bit silly to lock.
Just remove the locking altogether and get rid of both issues.
* Reduce scope of holding ServiceManager.planLock
In most cases we're just reading from m.plan fields (Services,
DefaultServiceNames, and so on), so we can use m.Plan(), which holds
the plan lock only for the duration of fetch m.plan -- this is safe,
because we never mutate what's inside m.plan, we only replace it (in
updatePlan).
For the cases we are updating the plan (AppendLayer, CombineLayer,
SetServiceArgs), we still need acquirePlan.
In ServiceManager.ServiceLogs, we don't need to acquire the plan lock
at all (m.plan isn't used).
feat(state): crossport latest changes from snapd/overlord/state (#344)
There are several major changes to the state package:
* Add WaitStatus support that allows a task to wait until further action
to continue its execution. The WaitStatus is treated mostly as
DoneStatus, except it is not ready status.
* Add Change.AbortUnreadyLanes.
* Add Change.CheckTaskDependencies to check if tasks have circular
dependencies.
* Add task and change callbacks invoked on a status change.
* Update clients of the State.Get to use a new NoStateError to check if
a desired key is present.
* Take StartOfOperationTime as a Prune parameter.
At the moment, the Pebble release process involves
a series of 3 commits under a very short amount of
time. GitHub's concurrency flag is being used to
cope with this, but it is unable to wait for
pending workflow runs, which raises the risk of
a Release workflow cancelling the previous
commit workflow, where the snap is being built,
if the run is pending, waiting for a GH runner.
This commit forces the snap build to happen on
releases also.