Remove lock options from the admin API (#2150)

api/admin/service.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"net/http"
 	"path"
+	"sync"
 
 	"github.com/gorilla/rpc/v2"
 
@@ -16,7 +17,6 @@ import (
 	"github.com/ava-labs/avalanchego/api/server"
 	"github.com/ava-labs/avalanchego/chains"
 	"github.com/ava-labs/avalanchego/ids"
-	"github.com/ava-labs/avalanchego/snow/engine/common"
 	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/ava-labs/avalanchego/utils/json"
@@ -53,23 +53,24 @@ type Config struct {
 // Admin is the API service for node admin management
 type Admin struct {
 	Config
+	lock     sync.RWMutex
 	profiler profiler.Profiler
 }
 
 // NewService returns a new admin API service.
 // All of the fields in [config] must be set.
-func NewService(config Config) (*common.HTTPHandler, error) {
-	newServer := rpc.NewServer()
+func NewService(config Config) (http.Handler, error) {
+	server := rpc.NewServer()
 	codec := json.NewCodec()
-	newServer.RegisterCodec(codec, "application/json")
-	newServer.RegisterCodec(codec, "application/json;charset=UTF-8")
-	if err := newServer.RegisterService(&Admin{
-		Config:   config,
-		profiler: profiler.New(config.ProfileDir),
-	}, "admin"); err != nil {
-		return nil, err
-	}
-	return &common.HTTPHandler{Handler: newServer}, nil
+	server.RegisterCodec(codec, "application/json")
+	server.RegisterCodec(codec, "application/json;charset=UTF-8")
+	return server, server.RegisterService(
+		&Admin{
+			Config:   config,
+			profiler: profiler.New(config.ProfileDir),
+		},
+		"admin",
+	)
 }
 
 // StartCPUProfiler starts a cpu profile writing to the specified file
@@ -79,6 +80,9 @@ func (a *Admin) StartCPUProfiler(_ *http.Request, _ *struct{}, _ *api.EmptyReply
 		zap.String("method", "startCPUProfiler"),
 	)
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	return a.profiler.StartCPUProfiler()
 }
 
@@ -89,6 +93,9 @@ func (a *Admin) StopCPUProfiler(_ *http.Request, _ *struct{}, _ *api.EmptyReply)
 		zap.String("method", "stopCPUProfiler"),
 	)
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	return a.profiler.StopCPUProfiler()
 }
 
@@ -99,6 +106,9 @@ func (a *Admin) MemoryProfile(_ *http.Request, _ *struct{}, _ *api.EmptyReply) e
 		zap.String("method", "memoryProfile"),
 	)
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	return a.profiler.MemoryProfile()
 }
 
@@ -109,6 +119,9 @@ func (a *Admin) LockProfile(_ *http.Request, _ *struct{}, _ *api.EmptyReply) err
 		zap.String("method", "lockProfile"),
 	)
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	return a.profiler.LockProfile()
 }
 
@@ -157,6 +170,9 @@ func (a *Admin) AliasChain(_ *http.Request, args *AliasChainArgs, _ *api.EmptyRe
 		return err
 	}
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	if err := a.ChainManager.Alias(chainID, args.Alias); err != nil {
 		return err
 	}
@@ -201,6 +217,10 @@ func (a *Admin) Stacktrace(_ *http.Request, _ *struct{}, _ *api.EmptyReply) erro
 	)
 
 	stacktrace := []byte(utils.GetStacktrace(true))
+
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	return perms.WriteFile(stacktraceFile, stacktrace, perms.ReadWrite)
 }
 
@@ -233,6 +253,9 @@ func (a *Admin) SetLoggerLevel(_ *http.Request, args *SetLoggerLevelArgs, _ *api
 		return errNoLogLevel
 	}
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	var loggerNames []string
 	if len(args.LoggerName) > 0 {
 		loggerNames = []string{args.LoggerName}
@@ -278,6 +301,10 @@ func (a *Admin) GetLoggerLevel(_ *http.Request, args *GetLoggerLevelArgs, reply
 		zap.String("method", "getLoggerLevels"),
 		logging.UserString("loggerName", args.LoggerName),
 	)
+
+	a.lock.RLock()
+	defer a.lock.RUnlock()
+
 	reply.LoggerLevels = make(map[string]LogAndDisplayLevels)
 	var loggerNames []string
 	// Empty name means all loggers
@@ -329,6 +356,9 @@ func (a *Admin) LoadVMs(r *http.Request, _ *struct{}, reply *LoadVMsReply) error
 		zap.String("method", "loadVMs"),
 	)
 
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	ctx := r.Context()
 	loadedVMs, failedVMs, err := a.VMRegistry.ReloadWithReadLock(ctx)
 	if err != nil {

node/node.go

@@ -1048,7 +1048,15 @@ func (n *Node) initAdminAPI() error {
 	if err != nil {
 		return err
 	}
-	return n.APIServer.AddRoute(service, &sync.RWMutex{}, "admin", "")
+	return n.APIServer.AddRoute(
+		&common.HTTPHandler{
+			LockOptions: common.NoLock,
+			Handler:     service,
+		},
+		&sync.RWMutex{},
+		"admin",
+		"",
+	)
 }
 
 // initProfiler initializes the continuous profiling