Merge lp:~chipaca/snappy/lock-ness into lp:~snappy-dev/snappy/snappy-moved-to-github

[John Lenton]

REST API now uses hierarchical package-grained locking.

Moo. TeX. What's not to like.

[Michael Vogt (mvo)]

Thanks for this branch! It looks good (but I need to read through it again as its quite complex at first). I got a bit of a meta-question. It seems like all the locking is done in the rest-api which is fine as its the only concurrent user of the API right now. But it also means that if someone builds on top of the snappy api he/she will have to rebuild the same locking. Is it a huge amount of work to put the locks inside snappy/* itself or is there another downside that I have not considered?

[Michael Vogt (mvo)]

Fwiw, I love the branch title :P

[John Lenton (chipaca)]

Remember the intention is to move away from having multiple things building against "the snappy api"; clients would use the rest api. There's a question about what that means for u-d-f, but locking shouldn't be an issue for it anyway.

The only downside to moving locking down in the call stack is that it's not clear where to lock, exactly, unless a lot of things grow a boolean "lock is already held" flag.

I can of course move the mmutex package one level down, but I don't think that's what you meant :)

[Sergio Schvezov (sergiusens)]

On Mon, Oct 19, 2015 at 6:47 AM, John Lenton <email address hidden>
wrote:

> Remember the intention is to move away from having multiple things
> building against "the snappy api"; clients would use the rest api. There's
> a question about what that means for u-d-f, but locking shouldn't be an
> issue for it anyway.
>

 The only thing u-d-f would need to do on its own is kernel, os and gadget
snap (bootstrapping problem); the apps and frameworks should be unpacked by
firstboot.

> The only downside to moving locking down in the call stack is that it's
> not clear where to lock, exactly, unless a lot of things grow a boolean
> "lock is already held" flag.
>

/me starts to type a reply
/me thinks http://cdn.meme.am/instances/500x/65135298.jpg

>
> I can of course move the mmutex package one level down, but I don't think
> that's what you meant :)
>
>

[Michael Vogt (mvo)]

Thanks for the branch and the explanation. One question inline and some minor stuff inline. I'm fine approving once the question is answered. In any case I read again tomorrow morning with a fresh mind, its not trivial (for me).

[Gustavo Niemeyer (niemeyer)]

Gustavo Niemeyer, [21.10.15 10:16]
@chipaca I'm lacking some context for why we'd want that locked-locking-locker

John Lenton, [21.10.15 10:17]
@niemeyer you mean the mutex inside the mmutex, or you mean the whole branch?

Gustavo Niemeyer, [21.10.15 10:17]
I mean the whole thing

John Lenton, [21.10.15 10:18]
@niemeyer well, we need some kind of a lock, to avoid people doing things at the same time and putting us in an unknown state

John Lenton, [21.10.15 10:19]
@niemeyer and a single, global lock didn't seem like a good idea

Gustavo Niemeyer, [21.10.15 10:19]
@chipaca In general we try to accommodate for people doing things at the same time instead, and prevent the actual critical things that cannot be mutated at once from being mutated at once

John Lenton, [21.10.15 10:19]
@niemeyer yes, in general we do

Gustavo Niemeyer, [21.10.15 10:20]
@chipaca Right, thus why I'm trying to understand how this locking system fits

John Lenton, [21.10.15 10:24]
@niemeyer it fits in that most of snappy is still the commandline one-shot thing, which has a filesystem-based lock by necessity to prevent different invocations of it to collide

John Lenton, [21.10.15 10:25]
@niemeyer a refactor of the level needed to move it to accomplish this exclusion without explicit locks is beyond the scope of the work we can do right now

Gustavo Niemeyer, [21.10.15 10:25]
@Chipaca So keep the filesystem locks?

Gustavo Niemeyer, [21.10.15 10:26]
@chipaca Being in a hurry and doing a big complex locking system doesn't feel compatible

John Lenton, [21.10.15 10:27]
this isn't a big complex locking system, imo

John Lenton, [21.10.15 10:27]
the filesystem lock is still there

Gustavo Niemeyer, [21.10.15 10:27]
@Chipaca Rather than one big central system, it would feel much better to me to give the proper places in the code authority for their own concurrency handling

John Lenton, [21.10.15 10:28]
@niemeyer isn't that what this does?

Gustavo Niemeyer, [21.10.15 10:29]
@chipaca That often amounts to a single local mutex, if at all

Gustavo Niemeyer, [21.10.15 10:29]
@chipaca So, again, I may well not be aware of the use case, but it feels like the use case is what we should be talking about

Gustavo Niemeyer, [21.10.15 10:30]
@chipaca It feels complex to me, and it feels complex to mvo, FWIW

John Lenton, [21.10.15 10:31]
yes, but less than 200 lines of code including whitespace and comments is not "big and complex"

John Lenton, [21.10.15 10:31]
anyway, the use case

Gustavo Niemeyer, [21.10.15 10:32]
@chipaca mutex.go is 126 lines long

Gustavo Niemeyer, [21.10.15 10:32]
and a lot of it is docs :)

Gustavo Niemeyer, [21.10.15 10:33]
But I digress

John Lenton, [21.10.15 10:33]
plus 137 for rwmutex

Gustavo Niemeyer, [21.10.15 10:33]
The key here is the use case.. if we need it, so be it, but I'm not yet understanding how this would fit in that cannot be solved by something simpler

John Lenton, [21.10.15 10:34]
when somebody we need a list of packages, or services, etc, we need to avoid people installing/removing/starting/stopping things for a bit

John Lenton, [21.10.15 10:34]
when people operate on a package, we need to stop people operating on that package for a b...

review: Needs Information

[Michael Vogt (mvo)]

Thanks for your replies, I replied inline as well.

Unmerged revisions

772. By John Lenton on 2015-10-15

Introducing the MMutex. It is not a cow in affordable clothing.

771. By John Lenton on 2015-10-14

Committing in search for a name

=== modified file 'cmd/snappy/common.go'
--- cmd/snappy/common.go	2015-07-09 06:05:53 +0000
+++ cmd/snappy/common.go	2015-10-15 11:44:27 +0000
@@ -20,17 +20,16 @@
 package main
 
 import (
+	"github.com/jessevdk/go-flags"
+
+	"launchpad.net/snappy/dirs"
 	"launchpad.net/snappy/logger"
 	"launchpad.net/snappy/priv"
-
-	"github.com/jessevdk/go-flags"
 )
 
-const snappyLockFile = "/run/snappy.lock"
-
 // withMutex runs the given function with a filelock mutex
 func withMutex(f func() error) error {
-	return priv.WithMutex(snappyLockFile, f)
+	return priv.WithMutex(dirs.SnapLockFile, f)
 }
 
 // addOptionDescription will try to find the given longName in the

=== modified file 'daemon/api.go'
--- daemon/api.go	2015-10-13 08:55:04 +0000
+++ daemon/api.go	2015-10-15 11:44:27 +0000
@@ -120,6 +120,9 @@
 )
 
 func v1Get(c *Command, r *http.Request) Response {
+	c.d.mmutex.RLock()
+	defer c.d.mmutex.RUnlock()
+
 	rel := release.Get()
 	m := map[string]string{
 		"flavor":          rel.Flavor,
@@ -156,6 +159,9 @@
 	name := vars["name"]
 	origin := vars["origin"]
 
+	c.d.mmutex.RLock(name, origin)
+	defer c.d.mmutex.RUnlock(name, origin)
+
 	repo := newRemoteRepo()
 	var part snappy.Part
 	if parts, _ := repo.Details(name, origin); len(parts) > 0 {
@@ -228,6 +234,9 @@
 		return InternalError(nil, "router can't find route for packages")
 	}
 
+	c.d.mmutex.RLock()
+	defer c.d.mmutex.RUnlock()
+
 	sources := make([]string, 1, 3)
 	sources[0] = "local"
 	// we're not worried if the remote repos error out
@@ -323,9 +332,18 @@
 		action = cmd["action"]
 	}
 
+	reachedAsync := false
 	switch action {
-	case "status", "start", "stop", "restart", "enable", "disable":
-		// ok
+	case "status":
+		c.d.mmutex.RLock(name, origin)
+		defer c.d.mmutex.RUnlock(name, origin)
+	case "start", "stop", "restart", "enable", "disable":
+		c.d.mmutex.Lock(name, origin)
+		defer func() {
+			if !reachedAsync {
+				c.d.mmutex.Unlock(name, origin)
+			}
+		}()
 	default:
 		return BadRequest(nil, "unknown action %s", action)
 	}
@@ -393,7 +411,11 @@
 		return SyncResponse(f())
 	}
 
+	reachedAsync = true
+
 	return AsyncResponse(c.d.AddTask(func() interface{} {
+		defer c.d.mmutex.Unlock(name, origin)
+
 		switch action {
 		case "start":
 			err = actor.Start()
@@ -425,6 +447,14 @@
 	}
 	pkgName := name + "." + origin
 
+	if r.Method == "GET" {
+		c.d.mmutex.RLock(name, origin)
+		defer c.d.mmutex.RUnlock(name, origin)
+	} else {
+		c.d.mmutex.Lock(name, origin)
+		defer c.d.mmutex.Unlock(name, origin)
+	}
+
 	bag := lightweight.PartBagByName(name, origin)
 	if bag == nil {
 		return NotFound
@@ -471,6 +501,9 @@
 	}
 
 	return AsyncResponse(c.d.AddTask(func() interface{} {
+		c.d.mmutex.Lock()
+		defer c.d.mmutex.Unlock()
+
 		rspmap := make(map[string]*configSubtask, len(pkgmap))
 		bags := lightweight.AllPartBags()
 		for pkg, cfg := range pkgmap {
@@ -653,7 +686,9 @@
 	}
 
 	vars := muxVars(r)
-	inst.pkg = vars["name"] + "." + vars["origin"]
+	name := vars["name"]
+	origin := vars["origin"]
+	inst.pkg = name + "." + origin
 	inst.prog = &progress.NullProgress{}
 
 	f := pkgActionDispatch(&inst)
@@ -661,7 +696,11 @@
 		return BadRequest(nil, "unknown action %s", inst.Action)
 	}
 
-	return AsyncResponse(c.d.AddTask(f).Map(route))
+	return AsyncResponse(c.d.AddTask(func() interface{} {
+		c.d.mmutex.Lock(name, origin)
+		defer c.d.mmutex.Unlock(name, origin)
+		return f()
+	}).Map(route))
 }
 
 const maxReadBuflen = 1024 * 1024
@@ -740,6 +779,9 @@
 			return err
 		}
 
+		c.d.mmutex.Lock()
+		defer c.d.mmutex.Unlock()
+
 		name, err := part.Install(&progress.NullProgress{}, 0)
 		if err != nil {
 			return err
@@ -752,8 +794,12 @@
 func getLogs(c *Command, r *http.Request) Response {
 	vars := muxVars(r)
 	name := vars["name"]
+	origin := vars["origin"]
 	svcName := vars["service"]
 
+	c.d.mmutex.RLock(name, origin)
+	defer c.d.mmutex.RUnlock(name, origin)
+
 	actor, err := findServices(name, svcName, &progress.NullProgress{})
 	if err != nil {
 		return NotFound(err, "no services found for %q: %v", name, err)
@@ -791,6 +837,9 @@
 	name := vars["name"]
 	origin := vars["origin"]
 
+	c.d.mmutex.RLock(name, origin)
+	defer c.d.mmutex.RUnlock(name, origin)
+
 	bag := lightweight.PartBagByName(name, origin)
 	if bag == nil || len(bag.Versions) == 0 {
 		return NotFound

=== modified file 'daemon/api_test.go'
--- daemon/api_test.go	2015-10-13 08:55:04 +0000
+++ daemon/api_test.go	2015-10-15 11:44:27 +0000
@@ -34,7 +34,6 @@
 	"os"
 	"path/filepath"
 	"strings"
-	"testing"
 	"time"
 
 	"gopkg.in/check.v1"
@@ -48,9 +47,6 @@
 	"launchpad.net/snappy/systemd"
 )
 
-// Hook up check.v1 into the "go test" runner
-func Test(t *testing.T) { check.TestingT(t) }
-
 type apiSuite struct {
 	parts []snappy.Part
 	err   error
@@ -75,6 +71,12 @@
 	return s.vars
 }
 
+// muLock is a minimal PrivLocker
+type muLock struct{}
+
+func (muLock) Lock() error   { return nil }
+func (muLock) Unlock() error { return nil }
+
 func (s *apiSuite) SetUpSuite(c *check.C) {
 	newRemoteRepo = func() metarepo {
 		return s
@@ -134,8 +136,7 @@
 }
 
 func (s *apiSuite) TestPackageInfoOneIntegration(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	s.vars = map[string]string{"name": "foo", "origin": "bar"}
 
@@ -217,8 +218,7 @@
 func (s *apiSuite) TestPackageInfoWeirdRoute(c *check.C) {
 	// can't really happen
 
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	// use the wrong command to force the issue
 	wrongCmd := &Command{Path: "/{what}", d: d}
@@ -230,8 +230,7 @@
 func (s *apiSuite) TestPackageInfoBadRoute(c *check.C) {
 	// can't really happen, v2
 
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	// get the route and break it
 	route := d.router.Get(packageCmd.Path)
@@ -374,8 +373,7 @@
 }
 
 func (s *apiSuite) TestPackagesInfoOnePerIntegration(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	req, err := http.NewRequest("GET", "/1.0/packages", nil)
 	c.Assert(err, check.IsNil)
@@ -416,8 +414,7 @@
 }
 
 func (s *apiSuite) TestDeleteOpNotFound(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	s.vars = map[string]string{"uuid": "42"}
 	rsp := deleteOp(operationCmd, nil).Self(nil, nil).(*resp)
@@ -426,8 +423,7 @@
 }
 
 func (s *apiSuite) TestDeleteOpStillRunning(c *check.C) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	d.tasks["42"] = &Task{}
 	s.vars = map[string]string{"uuid": "42"}
@@ -437,8 +433,7 @@
 }
 
 func (s *apiSuite) TestDeleteOp(c *check.C) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	task := &Task{}
 	d.tasks["42"] = task
@@ -450,8 +445,7 @@
 }
 
 func (s *apiSuite) TestGetOpInfoIntegration(c *check.C) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	s.vars = map[string]string{"uuid": "42"}
 	rsp := getOpInfo(operationCmd, nil).Self(nil, nil).(*resp)
@@ -504,8 +498,7 @@
 }
 
 func (s *apiSuite) TestPostPackageBadRequest(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	s.vars = map[string]string{"uuid": "42"}
 	rsp := getOpInfo(operationCmd, nil).Self(nil, nil).(*resp)
@@ -524,8 +517,7 @@
 }
 
 func (s *apiSuite) TestPostPackageBadAction(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	s.vars = map[string]string{"uuid": "42"}
 	c.Check(getOpInfo(operationCmd, nil).Self(nil, nil).(*resp).Status, check.Equals, http.StatusNotFound)
@@ -542,8 +534,7 @@
 }
 
 func (s *apiSuite) TestPostPackage(c *check.C) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	s.vars = map[string]string{"uuid": "42"}
 	c.Check(getOpInfo(operationCmd, nil).Self(nil, nil).(*resp).Status, check.Equals, http.StatusNotFound)
@@ -623,8 +614,7 @@
 }
 
 func (s *apiSuite) TestPackageGetConfig(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	req, err := http.NewRequest("GET", "/1.0/packages/foo.bar/config", bytes.NewBuffer(nil))
 	c.Assert(err, check.IsNil)
@@ -688,8 +678,7 @@
 }
 
 func (s *apiSuite) TestPackagePutConfig(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	newConfigStr := "some other config"
 	req, err := http.NewRequest("PUT", "/1.0/packages/foo.bar/config", bytes.NewBufferString(newConfigStr))
@@ -754,8 +743,7 @@
 }
 
 func (s *apiSuite) TestConfigMultiBadBody(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	req, err := http.NewRequest("PUT", "/1.0/packages", bytes.NewBuffer(nil))
 	c.Assert(err, check.IsNil)
@@ -785,8 +773,7 @@
 }
 
 func (s *apiSuite) genericTestPackagePut(c *check.C, body io.Reader, concreteNo int, expected map[string]*configSubtask) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	req, err := http.NewRequest("PUT", "/1.0/packages", body)
 	c.Assert(err, check.IsNil)
@@ -858,8 +845,7 @@
 }
 
 func (s *apiSuite) TestPackageServiceGet(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	findServices = func(string, string, progress.Meter) (snappy.ServiceActor, error) {
 		return &tSA{ssout: []*snappy.PackageServiceStatus{{ServiceName: "svc"}}}, nil
@@ -884,8 +870,7 @@
 }
 
 func (s *apiSuite) TestPackageServicePut(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	findServices = func(string, string, progress.Meter) (snappy.ServiceActor, error) {
 		return &tSA{ssout: []*snappy.PackageServiceStatus{{ServiceName: "svc"}}}, nil
@@ -953,8 +938,7 @@
 }
 
 func (s *apiSuite) TestServiceLogs(c *check.C) {
-	d := New()
-	d.addRoutes()
+	newTestDaemon()
 
 	log := systemd.Log{
 		"__REALTIME_TIMESTAMP": "42",

=== modified file 'daemon/daemon.go'
--- daemon/daemon.go	2015-10-12 14:55:27 +0000
+++ daemon/daemon.go	2015-10-15 11:44:27 +0000
@@ -31,6 +31,7 @@
 	"github.com/gorilla/mux"
 	"gopkg.in/tomb.v2"
 
+	"launchpad.net/snappy/daemon/mmutex"
 	"launchpad.net/snappy/logger"
 )
 
@@ -41,6 +42,7 @@
 	listener     net.Listener
 	tomb         tomb.Tomb
 	router       *mux.Router
+	mmutex       mmutex.MMutex
 }
 
 // A ResponseFunc handles one of the individual verbs for a method
@@ -124,6 +126,8 @@
 
 	d.addRoutes()
 
+	d.mmutex = mmutex.New()
+
 	logger.Debugf("init done in %s", time.Now().Sub(t0))
 
 	return nil

=== modified file 'daemon/daemon_test.go'
--- daemon/daemon_test.go	2015-10-05 23:10:59 +0000
+++ daemon/daemon_test.go	2015-10-15 11:44:27 +0000
@@ -23,15 +23,37 @@
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"testing"
 
 	"github.com/gorilla/mux"
 	"gopkg.in/check.v1"
 )
 
+// Hook up check.v1 into the "go test" runner
+func Test(t *testing.T) { check.TestingT(t) }
+
 type daemonSuite struct{}
 
 var _ = check.Suite(&daemonSuite{})
 
+// nopMMutex satisfies mmutex.MMutex but does nothing
+type nopMMutex struct{}
+
+func (nopMMutex) Lock(...string)    {}
+func (nopMMutex) RLock(...string)   {}
+func (nopMMutex) Unlock(...string)  {}
+func (nopMMutex) RUnlock(...string) {}
+
+// build a new daemon, with only a little of Init(), suitable for the tests
+func newTestDaemon() *Daemon {
+	d := New()
+	d.addRoutes()
+	d.mmutex = nopMMutex{}
+
+	return d
+}
+
+// aResponse suitable for testing
 type mockHandler struct {
 	cmd        *Command
 	lastMethod string
@@ -75,8 +97,7 @@
 }
 
 func (s *daemonSuite) TestAddRoutes(c *check.C) {
-	d := New()
-	d.addRoutes()
+	d := newTestDaemon()
 
 	expected := make([]string, len(api))
 	for i, v := range api {

=== added directory 'daemon/mmutex'
=== added file 'daemon/mmutex/mmutex.go'
--- daemon/mmutex/mmutex.go	1970-01-01 00:00:00 +0000
+++ daemon/mmutex/mmutex.go	2015-10-15 11:44:27 +0000
@@ -0,0 +1,213 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package mmutex
+
+import (
+	"strings"
+	"sync"
+
+	"launchpad.net/snappy/dirs"
+	"launchpad.net/snappy/priv"
+)
+
+/*
+
+(R)Lock:
+
+1. lock self
+2. lock global:
+  a. globalCount == 0? actually lock global
+  b. increase globalCount
+3. get locks for "" and key
+4. unlock self
+5. key != "" ? -> RLock ""
+6. (R)Lock key
+
+(R)Unlock:
+
+1. lock self
+2. (R)Unlock key
+3. key != "" ? -> RUnlock ""
+4. unlock global:
+  a. decrease globalCount
+  b. 0 globalCount? actually unlock global
+5. unlock self
+
+*/
+
+// privLocker is the interface of priv.Mutex that we use
+type privLocker interface {
+	Lock() error
+	Unlock() error
+}
+
+// newGlock calls priv.New; override it in tests to not panic when not
+// root.
+var newGlock = func() privLocker {
+	return priv.New(dirs.SnapLockFile)
+}
+
+type node struct {
+	sync.RWMutex
+	count uint
+}
+
+// A MMutex is a map of mutexes, with a special "root" mutex that lets you
+// Lock or RLock any of the non-root mutexes, but if the root mutex is
+// Locked you'll have to wait for it.
+type MMutex interface {
+	Lock(...string)
+	RLock(...string)
+	Unlock(...string)
+	RUnlock(...string)
+}
+
+type mmutex struct {
+	mutex   sync.Mutex // who locks the locker? This guy.
+	nodemap map[string]*node
+	glock   privLocker
+	gcount  uint
+}
+
+// New is the mmutex constructor.
+func New() MMutex {
+	return &mmutex{
+		nodemap: make(map[string]*node),
+	}
+}
+
+// acquires/increases count of the global lock
+// global lock only needed to interop with non-rest-api snappy
+func (lt *mmutex) lockGlobal() {
+	if lt.gcount == 0 {
+		lt.glock = newGlock()
+		if err := lt.glock.Lock(); err != nil {
+			panic(err)
+		}
+	}
+	lt.gcount++
+}
+
+// decreases count and releases the global lock
+// global lock only needed to interop with non-rest-api snappy
+func (lt *mmutex) unlockGlobal() {
+	lt.gcount--
+	if lt.gcount == 0 {
+		if err := lt.glock.Unlock(); err != nil {
+			panic(err)
+		}
+		// priv's Unlock sets the internal, private lock to nil;
+		// might as well get rid of the lock ourselves as it's
+		// useless at this point.
+		lt.glock = nil
+	}
+}
+
+// convenience autovivifying getter
+func (lt *mmutex) get(key string) *node {
+	if _, ok := lt.nodemap[key]; !ok {
+		lt.nodemap[key] = &node{}
+	}
+
+	return lt.nodemap[key]
+}
+
+// get the "root" lock (lock for "") and the lock for key
+func (lt *mmutex) rootNNode(key string) (root *node, node *node) {
+	lt.mutex.Lock()
+	defer lt.mutex.Unlock()
+
+	lt.lockGlobal()
+
+	root = lt.get("")
+	node = lt.get(key)
+
+	if key != "" {
+		node.count++
+	}
+
+	return root, node
+}
+
+// Lock the mutex for the given key. If the optional args are given, the
+// root mutex is RLocked; otherwise, the root mutex itself is Locked.
+func (lt *mmutex) Lock(args ...string) {
+	key := strings.Join(args, ".")
+	root, node := lt.rootNNode(key)
+
+	if key != "" {
+		root.RLock()
+	}
+
+	node.Lock()
+}
+
+// Unlock the mutex for the given key, and RUnlock the root mutex if args
+// are given.
+func (lt *mmutex) Unlock(args ...string) {
+	key := strings.Join(args, ".")
+	lt.mutex.Lock()
+	defer lt.mutex.Unlock()
+
+	node := lt.get(key)
+	node.Unlock()
+	if key != "" {
+		lt.get("").RUnlock()
+		node.count--
+		if node.count == 0 {
+			delete(lt.nodemap, key)
+		}
+	}
+
+	lt.unlockGlobal()
+}
+
+// RLock the mutex for the given key; also RLock the root mutex if args are
+// given.
+func (lt *mmutex) RLock(args ...string) {
+	key := strings.Join(args, ".")
+	root, node := lt.rootNNode(key)
+
+	if key != "" {
+		root.RLock()
+	}
+
+	node.RLock()
+}
+
+// RUnlock the mutex for the given key; also RUnlock the root mutex if args
+// are given.
+func (lt *mmutex) RUnlock(args ...string) {
+	key := strings.Join(args, ".")
+	lt.mutex.Lock()
+	defer lt.mutex.Unlock()
+
+	node := lt.get(key)
+	node.RUnlock()
+	if key != "" {
+		lt.get("").RUnlock()
+		node.count--
+		if node.count == 0 {
+			delete(lt.nodemap, key)
+		}
+	}
+
+	lt.unlockGlobal()
+}

=== added file 'daemon/mmutex/mmutex_test.go'
--- daemon/mmutex/mmutex_test.go	1970-01-01 00:00:00 +0000
+++ daemon/mmutex/mmutex_test.go	2015-10-15 11:44:27 +0000
@@ -0,0 +1,274 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package mmutex
+
+import (
+	"errors"
+	"sync"
+	"testing"
+	"time"
+
+	"gopkg.in/check.v1"
+)
+
+// Hook up check.v1 into the "go test" runner
+func Test(t *testing.T) { check.TestingT(t) }
+
+type mmutexSuite struct {
+	oldGlock func() privLocker
+	l        *muLck
+}
+
+var _ = check.Suite(&mmutexSuite{})
+
+type muLck struct {
+	m sync.Mutex
+	e error
+}
+
+func (mu *muLck) Lock() error   { mu.m.Lock(); return mu.e }
+func (mu *muLck) Unlock() error { mu.m.Unlock(); return mu.e }
+
+func (s *mmutexSuite) SetUpTest(c *check.C) {
+	s.oldGlock = newGlock
+	s.l = &muLck{}
+	newGlock = func() privLocker {
+		return s.l
+	}
+}
+
+func (s *mmutexSuite) TearDownTest(c *check.C) {
+	newGlock = s.oldGlock
+}
+
+func (s *mmutexSuite) TestSimultaneousLocks(c *check.C) {
+	ch := make(chan bool)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.Lock("foo")
+		defer mm.Unlock("foo")
+		mm.Lock("bar")
+		defer mm.Unlock("bar")
+		c.Check(mm.nodemap, check.HasLen, 3)
+		c.Check(mm.nodemap[""], check.NotNil)
+		c.Check(mm.nodemap["foo"], check.NotNil)
+		c.Check(mm.nodemap["bar"], check.NotNil)
+
+		ch <- true
+	}()
+	select {
+	case <-time.After(time.Second):
+		c.Fatalf("timed out")
+	case <-ch:
+	}
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestSimultaneousRLocks(c *check.C) {
+	ch := make(chan bool)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.RLock("foo")
+		defer mm.RUnlock("foo")
+		mm.RLock("bar")
+		defer mm.RUnlock("bar")
+		c.Check(mm.nodemap, check.HasLen, 3)
+		c.Check(mm.nodemap[""], check.NotNil)
+		c.Check(mm.nodemap["foo"], check.NotNil)
+		c.Check(mm.nodemap["bar"], check.NotNil)
+
+		ch <- true
+	}()
+	select {
+	case <-time.After(time.Second):
+		c.Fatalf("timed out")
+	case <-ch:
+	}
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestSimultaneousRLockWRoot(c *check.C) {
+	ch := make(chan bool)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.RLock()
+		defer mm.RUnlock()
+		mm.RLock("foo")
+		defer mm.RUnlock("foo")
+		c.Check(mm.nodemap, check.HasLen, 2)
+		c.Check(mm.nodemap[""], check.NotNil)
+		c.Check(mm.nodemap["foo"], check.NotNil)
+
+		ch <- true
+	}()
+	select {
+	case <-time.After(time.Second):
+		c.Fatalf("timed out")
+	case <-ch:
+	}
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestConcurrentRootLockWRLock(c *check.C) {
+	ch := make(chan bool)
+	buf := make(chan bool, 10)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.RLock("foo")
+		<-ch
+		defer mm.RUnlock("foo")
+
+		time.Sleep(100 * time.Millisecond)
+
+		buf <- false
+	}()
+
+	ch <- true // waits for mm.RLock("foo")
+
+	go func() {
+		mm.Lock()
+		defer mm.Unlock()
+
+		buf <- true
+	}()
+
+	c.Check(<-buf, check.Equals, false)
+	c.Check(<-buf, check.Equals, true)
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestConcurrentRLockWRootLock(c *check.C) {
+	ch := make(chan bool)
+	buf := make(chan bool, 10)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.Lock()
+		defer mm.Unlock()
+		<-ch
+
+		time.Sleep(100 * time.Millisecond)
+
+		buf <- true
+	}()
+
+	ch <- true // waits for mm.Lock()
+
+	go func() {
+		mm.RLock("foo")
+		defer mm.RUnlock("foo")
+
+		buf <- false
+	}()
+
+	c.Check(<-buf, check.Equals, true)
+	c.Check(<-buf, check.Equals, false)
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestConcurrentRootLockWLock(c *check.C) {
+	ch := make(chan bool)
+	buf := make(chan bool, 10)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.Lock("foo")
+		<-ch
+		defer mm.Unlock("foo")
+
+		time.Sleep(100 * time.Millisecond)
+
+		buf <- false
+	}()
+
+	ch <- true // waits for mm.Lock("foo")
+
+	go func() {
+		mm.Lock()
+		defer mm.Unlock()
+
+		buf <- true
+	}()
+
+	c.Check(<-buf, check.Equals, false)
+	c.Check(<-buf, check.Equals, true)
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestConcurrentLockWRootLock(c *check.C) {
+	ch := make(chan bool)
+	buf := make(chan bool, 10)
+
+	mm := New().(*mmutex)
+	go func() {
+		mm.Lock()
+		<-ch
+		defer mm.Unlock()
+
+		time.Sleep(100 * time.Millisecond)
+
+		buf <- true
+	}()
+
+	ch <- true // waits for mm.Lock()
+
+	go func() {
+		mm.Lock("foo")
+		defer mm.Unlock("foo")
+
+		buf <- false
+	}()
+
+	c.Check(<-buf, check.Equals, true)
+	c.Check(<-buf, check.Equals, false)
+
+	c.Check(mm.nodemap, check.HasLen, 1)
+	c.Check(mm.nodemap[""], check.NotNil)
+}
+
+func (s *mmutexSuite) TestLockPanicsIfGlobalLockFails(c *check.C) {
+	s.l.e = errors.New("failed")
+	mm := New().(*mmutex)
+	c.Check(func() { mm.Lock() }, check.Panics, s.l.e)
+}
+
+func (s *mmutexSuite) TestUnlockPanicsIfGlobalUnlockFails(c *check.C) {
+	mm := New().(*mmutex)
+	mm.Lock()
+	s.l.e = errors.New("failed")
+
+	c.Check(func() { mm.Unlock() }, check.Panics, s.l.e)
+}

=== modified file 'dirs/dirs.go'
--- dirs/dirs.go	2015-09-25 15:27:11 +0000
+++ dirs/dirs.go	2015-10-15 11:44:27 +0000
@@ -35,6 +35,7 @@
 	LocaleDir        string
 	SnapIconsDir     string
 	SnapMetaDir      string
+	SnapLockFile     string
 
 	SnapBinariesDir  string
 	SnapServicesDir  string
@@ -59,6 +60,7 @@
 	SnapSeccompDir = filepath.Join(rootdir, SnappyDir, "seccomp", "profiles")
 	SnapIconsDir = filepath.Join(rootdir, SnappyDir, "icons")
 	SnapMetaDir = filepath.Join(rootdir, SnappyDir, "meta")
+	SnapLockFile = filepath.Join(rootdir, "/run/snappy.lock")
 
 	SnapBinariesDir = filepath.Join(SnapAppsDir, "bin")
 	SnapServicesDir = filepath.Join(rootdir, "/etc/systemd/system")