Merge pull request #1026 from ellemouton/sql26

lit: make sure to always close `stores` on errored start-up

Author: guggero

config_dev.go

@@ -87,9 +87,9 @@ func defaultDevConfig() *DevConfig {
 func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
 	var (
 		networkDir = filepath.Join(cfg.LitDir, cfg.Network)
-		acctStore  accounts.Store
-		sessStore  session.Store
-		closeFns   = make(map[string]func() error)
+		stores     = &stores{
+			closeFns: make(map[string]func() error),
+		}
 	)
 
 	switch cfg.DatabaseBackend {
@@ -98,76 +98,68 @@ func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
 		// the directory where we will store the database file exists.
 		err := makeDirectories(networkDir)
 		if err != nil {
-			return nil, err
+			return stores, err
 		}
 
 		sqlStore, err := db.NewSqliteStore(cfg.Sqlite)
 		if err != nil {
-			return nil, err
+			return stores, err
 		}
 
-		acctStore = accounts.NewSQLStore(sqlStore.BaseDB, clock)
-		sessStore = session.NewSQLStore(sqlStore.BaseDB, clock)
-		closeFns["sqlite"] = sqlStore.BaseDB.Close
+		acctStore := accounts.NewSQLStore(sqlStore.BaseDB, clock)
+		sessStore := session.NewSQLStore(sqlStore.BaseDB, clock)
+
+		stores.accounts = acctStore
+		stores.sessions = sessStore
+		stores.closeFns["sqlite"] = sqlStore.BaseDB.Close
 
 	case DatabaseBackendPostgres:
 		sqlStore, err := db.NewPostgresStore(cfg.Postgres)
 		if err != nil {
-			return nil, err
+			return stores, err
 		}
 
-		acctStore = accounts.NewSQLStore(sqlStore.BaseDB, clock)
-		sessStore = session.NewSQLStore(sqlStore.BaseDB, clock)
-		closeFns["postgres"] = sqlStore.BaseDB.Close
+		acctStore := accounts.NewSQLStore(sqlStore.BaseDB, clock)
+		sessStore := session.NewSQLStore(sqlStore.BaseDB, clock)
+
+		stores.accounts = acctStore
+		stores.sessions = sessStore
+		stores.closeFns["postgres"] = sqlStore.BaseDB.Close
 
 	default:
 		accountStore, err := accounts.NewBoltStore(
 			filepath.Dir(cfg.MacaroonPath), accounts.DBFilename,
 			clock,
 		)
 		if err != nil {
-			return nil, err
+			return stores, err
 		}
-		closeFns["bbolt-accounts"] = accountStore.Close
+
+		stores.accounts = accountStore
+		stores.closeFns["bbolt-accounts"] = accountStore.Close
 
 		sessionStore, err := session.NewDB(
 			networkDir, session.DBFilename, clock, accountStore,
 		)
 		if err != nil {
-			return nil, err
+			return stores, err
 		}
-		closeFns["bbolt-sessions"] = sessionStore.Close
 
-		acctStore = accountStore
-		sessStore = sessionStore
+		stores.sessions = sessionStore
+		stores.closeFns["bbolt-sessions"] = sessionStore.Close
 	}
 
 	firewallBoltDB, err := firewalldb.NewBoltDB(
-		networkDir, firewalldb.DBFilename, sessStore,
+		networkDir, firewalldb.DBFilename, stores.sessions,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("error creating firewall BoltDB: %v",
+		return stores, fmt.Errorf("error creating firewall BoltDB: %v",
 			err)
 	}
-	closeFns["bbolt-firewalldb"] = firewallBoltDB.Close
-
-	return &stores{
-		accounts:     acctStore,
-		sessions:     sessStore,
-		firewall:     firewalldb.NewDB(firewallBoltDB),
-		firewallBolt: firewallBoltDB,
-		close: func() error {
-			var returnErr error
-			for storeName, fn := range closeFns {
-				err := fn()
-				if err != nil {
-					log.Errorf("error closing %s store: %v",
-						storeName, err)
-					returnErr = err
-				}
-			}
-
-			return returnErr
-		},
-	}, nil
+
+	stores.firewall = firewalldb.NewDB(firewallBoltDB)
+	stores.firewallBolt = firewallBoltDB
+	stores.closeFns["bbolt-firewalldb"] = firewallBoltDB.Close
+
+	return stores, nil
 }

config_prod.go

@@ -32,55 +32,38 @@ func (c *DevConfig) Validate(_, _ string) error {
 func NewStores(cfg *Config, clock clock.Clock) (*stores, error) {
 	networkDir := filepath.Join(cfg.LitDir, cfg.Network)
 
+	stores := &stores{
+		closeFns: make(map[string]func() error),
+	}
+
 	acctStore, err := accounts.NewBoltStore(
 		filepath.Dir(cfg.MacaroonPath), accounts.DBFilename, clock,
 	)
 	if err != nil {
-		return nil, err
+		return stores, err
 	}
+	stores.accounts = acctStore
+	stores.closeFns["accounts"] = acctStore.Close
 
 	sessStore, err := session.NewDB(
 		networkDir, session.DBFilename, clock, acctStore,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("error creating session BoltStore: %v",
-			err)
+		return stores, fmt.Errorf("error creating session BoltStore: "+
+			"%v", err)
 	}
+	stores.sessions = sessStore
+	stores.closeFns["sessions"] = sessStore.Close
 
 	firewallDB, err := firewalldb.NewBoltDB(
 		networkDir, firewalldb.DBFilename, sessStore,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("error creating firewall DB: %v", err)
+		return stores, fmt.Errorf("error creating firewall DB: %v", err)
 	}
+	stores.firewallBolt = firewallDB
+	stores.firewall = firewalldb.NewDB(firewallDB)
+	stores.closeFns["firewall"] = firewallDB.Close
 
-	return &stores{
-		accounts:     acctStore,
-		sessions:     sessStore,
-		firewallBolt: firewallDB,
-		firewall:     firewalldb.NewDB(firewallDB),
-		close: func() error {
-			var returnErr error
-			if err := acctStore.Close(); err != nil {
-				returnErr = fmt.Errorf("error closing "+
-					"account store: %v", err)
-
-				log.Error(returnErr.Error())
-			}
-			if err := sessStore.Close(); err != nil {
-				returnErr = fmt.Errorf("error closing "+
-					"session store: %v", err)
-
-				log.Error(returnErr.Error())
-			}
-			if err := firewallDB.Close(); err != nil {
-				returnErr = fmt.Errorf("error closing "+
-					"firewall DB: %v", err)
-
-				log.Error(returnErr.Error())
-			}
-
-			return returnErr
-		},
-	}, nil
+	return stores, nil
 }

terminal.go

@@ -242,9 +242,25 @@ type stores struct {
 	firewall     *firewalldb.DB
 	firewallBolt *firewalldb.BoltDB
 
-	// close is a callback that can be used to close all the stores in the
-	// stores struct.
-	close func() error
+	// closeFns holds various callbacks that can be used to close any open
+	// stores in the stores struct.
+	closeFns map[string]func() error
+}
+
+// close closes all open stores in the stores struct. It returns an error if
+// any of the stores could not be closed.
+func (s *stores) close() error {
+	var returnErr error
+	for storeName, closeFn := range s.closeFns {
+		err := closeFn()
+		if err != nil {
+			log.Errorf("error closing %s store: %v",
+				storeName, err)
+			returnErr = err
+		}
+	}
+
+	return returnErr
 }
 
 // Run starts everything and then blocks until either the application is shut
@@ -1450,9 +1466,13 @@ func (g *LightningTerminal) shutdownSubServers() error {
 	}
 
 	if g.stores != nil {
-		if err := g.stores.firewall.Stop(); err != nil {
-			log.Errorf("Error stoppint firewall DB: %v", err)
-			returnErr = err
+		if g.stores.firewall != nil {
+			if err := g.stores.firewall.Stop(); err != nil {
+				log.Errorf("Error stoppint firewall DB: %v",
+					err)
+
+				returnErr = err
+			}
 		}
 
 		err = g.stores.close()