From 7398d737b5f52e32fbc8d1cd66ce55374d74c34b Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Thu, 1 Sep 2016 20:13:11 -0700
Subject: [PATCH] profiler: Fix it properly and generate/save profiles even
 failure situations. (#2607)

Fixes #2594
---
 cmd/main.go  | 22 +++++-----------------
 cmd/utils.go | 49 +++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/cmd/main.go b/cmd/main.go
index 503f7d42c..578aefd70 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -23,7 +23,6 @@ import (
 
 	"github.com/minio/cli"
 	"github.com/minio/mc/pkg/console"
-	"github.com/pkg/profile"
 )
 
 var (
@@ -130,6 +129,7 @@ func registerApp() *cli.App {
 	return app
 }
 
+// Verify main command syntax.
 func checkMainSyntax(c *cli.Context) {
 	configPath, err := getConfigPath()
 	if err != nil {
@@ -140,12 +140,7 @@ func checkMainSyntax(c *cli.Context) {
 	}
 }
 
-// Global profiler to be cleanly set and saved inside graceful shutdown.
-var globalProfiler interface {
-	Stop()
-}
-
-// Main - main for minio server.
+// Main main for minio server.
 func Main() {
 	app := registerApp()
 	app.Before = func(c *cli.Context) error {
@@ -181,16 +176,9 @@ func Main() {
 		return nil
 	}
 
-	// Set ``MINIO_PROFILE_DIR`` to the directory where profiling information should be persisted
-	profileDir := os.Getenv("MINIO_PROFILE_DIR")
-	// Enable profiler if ``MINIO_PROFILER`` is set. Supported options are [cpu, mem, block].
-	switch os.Getenv("MINIO_PROFILER") {
-	case "cpu":
-		globalProfiler = profile.Start(profile.CPUProfile, profile.ProfilePath(profileDir))
-	case "mem":
-		globalProfiler = profile.Start(profile.MemProfile, profile.ProfilePath(profileDir))
-	case "block":
-		globalProfiler = profile.Start(profile.BlockProfile, profile.ProfilePath(profileDir))
+	// Start profiler if env is set.
+	if profiler := os.Getenv("MINIO_PROFILER"); profiler != "" {
+		globalProfiler = startProfiler(profiler)
 	}
 
 	// Run the app - exit on error.
diff --git a/cmd/utils.go b/cmd/utils.go
index 2067e977f..fa28d7803 100644
--- a/cmd/utils.go
+++ b/cmd/utils.go
@@ -26,6 +26,8 @@ import (
 	"strings"
 	"sync"
 	"syscall"
+
+	"github.com/pkg/profile"
 )
 
 // xmlDecoder provide decoded value in xml.
@@ -154,17 +156,44 @@ const (
 	shutdownRestart
 )
 
+// Starts a profiler returns nil if profiler is not enabled, caller needs to handle this.
+func startProfiler(profiler string) interface {
+	Stop()
+} {
+	// Set ``MINIO_PROFILE_DIR`` to the directory where profiling information should be persisted
+	profileDir := os.Getenv("MINIO_PROFILE_DIR")
+	// Enable profiler if ``MINIO_PROFILER`` is set. Supported options are [cpu, mem, block].
+	switch profiler {
+	case "cpu":
+		return profile.Start(profile.CPUProfile, profile.NoShutdownHook, profile.ProfilePath(profileDir))
+	case "mem":
+		return profile.Start(profile.MemProfile, profile.NoShutdownHook, profile.ProfilePath(profileDir))
+	case "block":
+		return profile.Start(profile.BlockProfile, profile.NoShutdownHook, profile.ProfilePath(profileDir))
+	default:
+		return nil
+	}
+}
+
 // Global shutdown signal channel.
 var globalShutdownSignalCh = make(chan shutdownSignal, 1)
 
+// Global profiler to be used by shutdown go-routine.
+var globalProfiler interface {
+	Stop()
+}
+
 // Start to monitor shutdownSignal to execute shutdown callbacks
 func startMonitorShutdownSignal(onExitFn onExitFunc) error {
 	// Validate exit func.
 	if onExitFn == nil {
 		return errInvalidArgument
 	}
+
+	// Start listening on shutdown signal.
 	go func() {
 		defer close(globalShutdownSignalCh)
+
 		// Monitor signals.
 		trapCh := signalTrap(os.Interrupt, syscall.SIGTERM)
 		for {
@@ -177,6 +206,10 @@ func startMonitorShutdownSignal(onExitFn onExitFunc) error {
 				for _, callback := range globalShutdownCBs.GetObjectLayerCBs() {
 					exitCode := callback()
 					if exitCode != exitSuccess {
+						// If global profiler is set stop before we exit.
+						if globalProfiler != nil {
+							globalProfiler.Stop()
+						}
 						onExitFn(int(exitCode))
 					}
 
@@ -185,6 +218,10 @@ func startMonitorShutdownSignal(onExitFn onExitFunc) error {
 				for _, callback := range globalShutdownCBs.GetGenericCBs() {
 					exitCode := callback()
 					if exitCode != exitSuccess {
+						// If global profiler is set stop before we exit.
+						if globalProfiler != nil {
+							globalProfiler.Stop()
+						}
 						onExitFn(int(exitCode))
 					}
 				}
@@ -201,14 +238,18 @@ func startMonitorShutdownSignal(onExitFn onExitFunc) error {
 					if err != nil {
 						errorIf(errors.New("Unable to reboot."), err.Error())
 					}
+
+					// If global profiler is set stop before we exit.
+					if globalProfiler != nil {
+						globalProfiler.Stop()
+					}
+
 					// Successfully forked.
 					onExitFn(int(exitSuccess))
 				}
 
-				// Enable profiler if ``MINIO_PROFILER`` is set.
-				switch os.Getenv("MINIO_PROFILER") {
-				case "cpu", "mem", "block":
-					// Stop any running profiler.
+				// If global profiler is set stop before we exit.
+				if globalProfiler != nil {
 					globalProfiler.Stop()
 				}