Merge pull request #9 from tendermint/bugfix/check-for-error-returned-by-impl

[service] check for error returned by impl

service.go

@@ -1,13 +1,40 @@
-/*
+package common
 
-Classical-inheritance-style service declarations.
-Services can be started, then stopped, then optionally restarted.
-Users can override the OnStart/OnStop methods.
-By default, these methods are guaranteed to be called at most once.
-A call to Reset will panic, unless OnReset is overwritten, allowing OnStart/OnStop to be called again.
-Caller must ensure that Start() and Stop() are not called concurrently.
-It is ok to call Stop() without calling Start() first.
-Services cannot be re-started unless OnReset is overwritten to allow it.
+import (
+	"sync/atomic"
+
+	"github.com/tendermint/log15"
+)
+
+type Service interface {
+	Start() (bool, error)
+	OnStart() error
+
+	Stop() bool
+	OnStop()
+
+	Reset() (bool, error)
+	OnReset() error
+
+	IsRunning() bool
+
+	String() string
+}
+
+/*
+Classical-inheritance-style service declarations. Services can be started, then
+stopped, then optionally restarted.
+
+Users can override the OnStart/OnStop methods. In the absence of errors, these
+methods are guaranteed to be called at most once. If OnStart returns an error,
+service won't be marked as started, so the user can call Start again.
+
+A call to Reset will panic, unless OnReset is overwritten, allowing
+OnStart/OnStop to be called again.
+
+The caller must ensure that Start and Stop are not called concurrently.
+
+It is ok to call Stop without calling Start first.
 
 Typical usage:
 
@@ -35,31 +62,7 @@ func (fs *FooService) OnStop() error {
 		// close/destroy private fields
 		// stop subroutines, etc.
 	}
-
 */
-package common
-
-import (
-	"sync/atomic"
-
-	"github.com/tendermint/log15"
-)
-
-type Service interface {
-	Start() (bool, error)
-	OnStart() error
-
-	Stop() bool
-	OnStop()
-
-	Reset() (bool, error)
-	OnReset() error
-
-	IsRunning() bool
-
-	String() string
-}
-
 type BaseService struct {
 	log     log15.Logger
 	name    string
@@ -94,6 +97,11 @@ func (bs *BaseService) Start() (bool, error) {
 			}
 		}
 		err := bs.impl.OnStart()
+		if err != nil {
+			// revert flag
+			atomic.StoreUint32(&bs.started, 0)
+			return false, err
+		}
 		return true, err
 	} else {
 		if bs.log != nil {