From ab0c668ab8bb4bb91fa497a185debb5d29df6a77 Mon Sep 17 00:00:00 2001
From: Jeromy <jeromyj@gmail.com>
Date: Thu, 3 Sep 2015 09:28:36 -0700
Subject: [PATCH] fix panic caused by accessing config after repo closed

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
---
 cmd/ipfs/daemon.go            |  8 +++++++-
 cmd/ipfswatch/main.go         |  2 +-
 core/builder.go               |  6 +++++-
 core/commands/bootstrap.go    | 18 +++++++++++++++---
 core/core.go                  | 30 ++++++++++++++++++++++++------
 core/corehttp/commands.go     |  6 +++++-
 core/corehttp/gateway.go      |  7 ++++++-
 fuse/ipns/mount_unix.go       |  6 +++++-
 fuse/readonly/mount_unix.go   |  5 ++++-
 repo/fsrepo/fsrepo.go         |  9 +++------
 repo/mock.go                  |  4 ++--
 repo/repo.go                  |  2 +-
 test/supernode_client/main.go |  8 ++++++--
 util/sadhack/godep.go         | 14 --------------
 14 files changed, 84 insertions(+), 41 deletions(-)
 delete mode 100644 util/sadhack/godep.go

diff --git a/cmd/ipfs/daemon.go b/cmd/ipfs/daemon.go
index 0a1134632..5f5765bb0 100644
--- a/cmd/ipfs/daemon.go
+++ b/cmd/ipfs/daemon.go
@@ -204,7 +204,13 @@ func daemonFunc(req cmds.Request, res cmds.Response) {
 		return
 	}
 	if routingOption == routingOptionSupernodeKwd {
-		servers, err := repo.Config().SupernodeRouting.ServerIPFSAddrs()
+		rcfg, err := repo.Config()
+		if err != nil {
+			res.SetError(err, cmds.ErrNormal)
+			return
+		}
+
+		servers, err := rcfg.SupernodeRouting.ServerIPFSAddrs()
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)
 			repo.Close() // because ownership hasn't been transferred to the node
diff --git a/cmd/ipfswatch/main.go b/cmd/ipfswatch/main.go
index eb52b7ce1..ec98f2bff 100644
--- a/cmd/ipfswatch/main.go
+++ b/cmd/ipfswatch/main.go
@@ -192,7 +192,7 @@ func cmdCtx(node *core.IpfsNode, repoPath string) commands.Context {
 		Online:     true,
 		ConfigRoot: repoPath,
 		LoadConfig: func(path string) (*config.Config, error) {
-			return node.Repo.Config(), nil
+			return node.Repo.Config()
 		},
 		ConstructNode: func() (*core.IpfsNode, error) {
 			return node, nil
diff --git a/core/builder.go b/core/builder.go
index ceddc42f8..999f11a46 100644
--- a/core/builder.go
+++ b/core/builder.go
@@ -135,7 +135,11 @@ func setupNode(ctx context.Context, n *IpfsNode, cfg *BuildCfg) error {
 	}
 
 	if cfg.Online {
-		do := setupDiscoveryOption(n.Repo.Config().Discovery)
+		rcfg, err := n.Repo.Config()
+		if err != nil {
+			return err
+		}
+		do := setupDiscoveryOption(rcfg.Discovery)
 		if err := n.startOnlineServices(ctx, cfg.Routing, cfg.Host, do); err != nil {
 			return err
 		}
diff --git a/core/commands/bootstrap.go b/core/commands/bootstrap.go
index c9134e6d4..f79d0f151 100644
--- a/core/commands/bootstrap.go
+++ b/core/commands/bootstrap.go
@@ -72,7 +72,11 @@ in the bootstrap list).
 			return
 		}
 		defer r.Close()
-		cfg := r.Config()
+		cfg, err := r.Config()
+		if err != nil {
+			res.SetError(err, cmds.ErrNormal)
+			return
+		}
 
 		deflt, _, err := req.Option("default").Bool()
 		if err != nil {
@@ -148,7 +152,11 @@ var bootstrapRemoveCmd = &cmds.Command{
 			return
 		}
 		defer r.Close()
-		cfg := r.Config()
+		cfg, err := r.Config()
+		if err != nil {
+			res.SetError(err, cmds.ErrNormal)
+			return
+		}
 
 		all, _, err := req.Option("all").Bool()
 		if err != nil {
@@ -197,7 +205,11 @@ var bootstrapListCmd = &cmds.Command{
 			return
 		}
 		defer r.Close()
-		cfg := r.Config()
+		cfg, err := r.Config()
+		if err != nil {
+			res.SetError(err, cmds.ErrNormal)
+			return
+		}
 
 		peers, err := cfg.BootstrapPeers()
 		if err != nil {
diff --git a/core/core.go b/core/core.go
index beabb6751..6b6dbd03f 100644
--- a/core/core.go
+++ b/core/core.go
@@ -136,7 +136,10 @@ func (n *IpfsNode) startOnlineServices(ctx context.Context, routingOption Routin
 	n.Reporter = metrics.NewBandwidthCounter()
 
 	// get undialable addrs from config
-	cfg := n.Repo.Config()
+	cfg, err := n.Repo.Config()
+	if err != nil {
+		return err
+	}
 	var addrfilter []*net.IPNet
 	for _, s := range cfg.Swarm.AddrFilters {
 		f, err := mamask.NewMask(s)
@@ -156,7 +159,7 @@ func (n *IpfsNode) startOnlineServices(ctx context.Context, routingOption Routin
 	}
 
 	// Ok, now we're ready to listen.
-	if err := startListening(ctx, n.PeerHost, n.Repo.Config()); err != nil {
+	if err := startListening(ctx, n.PeerHost, cfg); err != nil {
 		return err
 	}
 
@@ -325,7 +328,7 @@ func (n *IpfsNode) Bootstrap(cfg BootstrapConfig) error {
 		cfg.BootstrapPeers = func() []peer.PeerInfo {
 			ps, err := n.loadBootstrapPeers()
 			if err != nil {
-				log.Warningf("failed to parse bootstrap peers from config: %s", n.Repo.Config().Bootstrap)
+				log.Warning("failed to parse bootstrap peers from config")
 				return nil
 			}
 			return ps
@@ -342,7 +345,12 @@ func (n *IpfsNode) loadID() error {
 		return errors.New("identity already loaded")
 	}
 
-	cid := n.Repo.Config().Identity.PeerID
+	cfg, err := n.Repo.Config()
+	if err != nil {
+		return err
+	}
+
+	cid := cfg.Identity.PeerID
 	if cid == "" {
 		return errors.New("Identity was not set in config (was ipfs init run?)")
 	}
@@ -363,7 +371,12 @@ func (n *IpfsNode) LoadPrivateKey() error {
 		return errors.New("private key already loaded")
 	}
 
-	sk, err := loadPrivateKey(&n.Repo.Config().Identity, n.Identity)
+	cfg, err := n.Repo.Config()
+	if err != nil {
+		return err
+	}
+
+	sk, err := loadPrivateKey(&cfg.Identity, n.Identity)
 	if err != nil {
 		return err
 	}
@@ -375,7 +388,12 @@ func (n *IpfsNode) LoadPrivateKey() error {
 }
 
 func (n *IpfsNode) loadBootstrapPeers() ([]peer.PeerInfo, error) {
-	parsed, err := n.Repo.Config().BootstrapPeers()
+	cfg, err := n.Repo.Config()
+	if err != nil {
+		return nil, err
+	}
+
+	parsed, err := cfg.BootstrapPeers()
 	if err != nil {
 		return nil, err
 	}
diff --git a/core/corehttp/commands.go b/core/corehttp/commands.go
index e3a64f0f8..99f544905 100644
--- a/core/corehttp/commands.go
+++ b/core/corehttp/commands.go
@@ -107,8 +107,12 @@ func commandsOption(cctx commands.Context, command *commands.Command) ServeOptio
 				AllowedMethods: []string{"GET", "POST", "PUT"},
 			},
 		}
+		rcfg, err := n.Repo.Config()
+		if err != nil {
+			return nil, err
+		}
 
-		addHeadersFromConfig(cfg, n.Repo.Config())
+		addHeadersFromConfig(cfg, rcfg)
 		addCORSFromEnv(cfg)
 		addCORSDefaults(cfg)
 		patchCORSVars(cfg, l.Addr())
diff --git a/core/corehttp/gateway.go b/core/corehttp/gateway.go
index 584a89437..2aa95b951 100644
--- a/core/corehttp/gateway.go
+++ b/core/corehttp/gateway.go
@@ -30,7 +30,12 @@ func NewGateway(conf GatewayConfig) *Gateway {
 func (g *Gateway) ServeOption() ServeOption {
 	return func(n *core.IpfsNode, _ net.Listener, mux *http.ServeMux) (*http.ServeMux, error) {
 		// pass user's HTTP headers
-		g.Config.Headers = n.Repo.Config().Gateway.HTTPHeaders
+		cfg, err := n.Repo.Config()
+		if err != nil {
+			return nil, err
+		}
+
+		g.Config.Headers = cfg.Gateway.HTTPHeaders
 
 		gateway, err := newGatewayHandler(n, g.Config)
 		if err != nil {
diff --git a/fuse/ipns/mount_unix.go b/fuse/ipns/mount_unix.go
index 5ec33eded..620ce9fa7 100644
--- a/fuse/ipns/mount_unix.go
+++ b/fuse/ipns/mount_unix.go
@@ -11,7 +11,11 @@ import (
 
 // Mount mounts ipns at a given location, and returns a mount.Mount instance.
 func Mount(ipfs *core.IpfsNode, ipnsmp, ipfsmp string) (mount.Mount, error) {
-	cfg := ipfs.Repo.Config()
+	cfg, err := ipfs.Repo.Config()
+	if err != nil {
+		return nil, err
+	}
+
 	allow_other := cfg.Mounts.FuseAllowOther
 
 	if ipfs.IpnsFs == nil {
diff --git a/fuse/readonly/mount_unix.go b/fuse/readonly/mount_unix.go
index dc5a56e4e..9ba1b6b6c 100644
--- a/fuse/readonly/mount_unix.go
+++ b/fuse/readonly/mount_unix.go
@@ -10,7 +10,10 @@ import (
 
 // Mount mounts ipfs at a given location, and returns a mount.Mount instance.
 func Mount(ipfs *core.IpfsNode, mountpoint string) (mount.Mount, error) {
-	cfg := ipfs.Repo.Config()
+	cfg, err := ipfs.Repo.Config()
+	if err != nil {
+		return nil, err
+	}
 	allow_other := cfg.Mounts.FuseAllowOther
 	fsys := NewFileSystem(ipfs)
 	return mount.NewMount(ipfs.Process(), fsys, mountpoint, allow_other)
diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go
index b3ea807ca..501580cf6 100644
--- a/repo/fsrepo/fsrepo.go
+++ b/repo/fsrepo/fsrepo.go
@@ -445,11 +445,8 @@ func (r *FSRepo) Close() error {
 	return nil
 }
 
-// Config returns the FSRepo's config. This method must not be called if the
-// repo is not open.
-//
 // Result when not Open is undefined. The method may panic if it pleases.
-func (r *FSRepo) Config() *config.Config {
+func (r *FSRepo) Config() (*config.Config, error) {
 
 	// It is not necessary to hold the package lock since the repo is in an
 	// opened state. The package lock is _not_ meant to ensure that the repo is
@@ -460,9 +457,9 @@ func (r *FSRepo) Config() *config.Config {
 	defer packageLock.Unlock()
 
 	if r.closed {
-		panic("repo is closed")
+		return nil, errors.New("cannot access config, repo not open")
 	}
-	return r.config
+	return r.config, nil
 }
 
 // setConfigUnsynced is for private use.
diff --git a/repo/mock.go b/repo/mock.go
index 7446eeff8..116ccefdc 100644
--- a/repo/mock.go
+++ b/repo/mock.go
@@ -15,8 +15,8 @@ type Mock struct {
 	D ds.ThreadSafeDatastore
 }
 
-func (m *Mock) Config() *config.Config {
-	return &m.C // FIXME threadsafety
+func (m *Mock) Config() (*config.Config, error) {
+	return &m.C, nil // FIXME threadsafety
 }
 
 func (m *Mock) SetConfig(updated *config.Config) error {
diff --git a/repo/repo.go b/repo/repo.go
index 2536bc15a..4efcb2eb6 100644
--- a/repo/repo.go
+++ b/repo/repo.go
@@ -14,7 +14,7 @@ var (
 )
 
 type Repo interface {
-	Config() *config.Config
+	Config() (*config.Config, error)
 	SetConfig(*config.Config) error
 
 	SetConfigKey(key string, value interface{}) error
diff --git a/test/supernode_client/main.go b/test/supernode_client/main.go
index a0150c232..92f4c9445 100644
--- a/test/supernode_client/main.go
+++ b/test/supernode_client/main.go
@@ -70,7 +70,11 @@ func run() error {
 	if err != nil { // owned by node
 		return err
 	}
-	cfg := repo.Config()
+	cfg, err := repo.Config()
+	if err != nil {
+		return err
+	}
+
 	cfg.Bootstrap = servers
 	if err := repo.SetConfig(cfg); err != nil {
 		return err
@@ -236,7 +240,7 @@ func cmdCtx(node *core.IpfsNode, repoPath string) commands.Context {
 		Online:     true,
 		ConfigRoot: repoPath,
 		LoadConfig: func(path string) (*config.Config, error) {
-			return node.Repo.Config(), nil
+			return node.Repo.Config()
 		},
 		ConstructNode: func() (*core.IpfsNode, error) {
 			return node, nil
diff --git a/util/sadhack/godep.go b/util/sadhack/godep.go
deleted file mode 100644
index 0ee9cf748..000000000
--- a/util/sadhack/godep.go
+++ /dev/null
@@ -1,14 +0,0 @@
-package util
-
-// FIXME: we need the go-random/random utility for our sharness test wich depends on go-humanize
-// Godep will drop it if we dont use it in ipfs. There should be a better way to do this.
-import _ "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/dustin/go-humanize"
-
-// similar to the above, only used in the tests makefile
-import _ "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/whyrusleeping/iptb"
-
-import _ "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/chriscool/go-sleep"
-
-// imported by chegga/pb on windows, this is here so running godeps on non-windows doesnt
-// drop it from our vendoring
-import _ "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/olekukonko/ts"