From df9247c00b05af8e47db7e1311cd7cce9c6341e2 Mon Sep 17 00:00:00 2001 From: Alexey Savchuk Date: Tue, 5 Nov 2024 10:52:36 +0300 Subject: [PATCH] services/rpcsrv: Return a new server by pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, a new server was returned by value which could cause a panic `unlock of unlocked mutex` on SIGHUP handling. It's because the new server overwrites a locked mutex of the already existing server. oct‚ 22 13:51:15 node1 neo-go[1183338]: fatal error: sync: Unlock of unlocked RWMutex oct‚ 22 13:51:15 node1 neo-go[1183338]: goroutine 538 [running]: oct‚ 22 13:51:15 node1 neo-go[1183338]: sync.fatal({0xf83d64?, 0xc001085880?}) oct‚ 22 13:51:15 node1 neo-go[1183338]: runtime/panic.go:1007 +0x18 oct‚ 22 13:51:15 node1 neo-go[1183338]: sync.(*RWMutex).Unlock(0xc00019a4c8) oct‚ 22 13:51:15 node1 neo-go[1183338]: sync/rwmutex.go:208 +0x45 oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv.(*Server).dropSubscriber(0xc00019a2c8, 0xc000a77740) oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/server.go:825 +0xce oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv.(*Server).handleWsReads(0xc00019a2c8, 0xc0034478c0, 0xc000af5f80, 0xc000a7 7740) oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/server.go:810 +0x266 oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv.(*Server).handleHTTPRequest(0xc00019a2c8, {0x11c3900, 0xc003437dc0}, 0xc00 31945a0) oct‚ 22 13:51:15 node1 neo-go[1183338]: github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/server.go:582 +0x54a oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http.HandlerFunc.ServeHTTP(0x471779?, {0x11c3900?, 0xc003437dc0?}, 0xc000943b68?) oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http/server.go:2171 +0x29 oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http.serverHandler.ServeHTTP({0xc000a77680?}, {0x11c3900?, 0xc003437dc0?}, 0x6?) oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http/server.go:3142 +0x8e oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http.(*conn).serve(0xc0032030e0, {0x11c5220, 0xc000a76960}) oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http/server.go:2044 +0x5e8 oct‚ 22 13:51:15 node1 neo-go[1183338]: created by net/http.(*Server).Serve in goroutine 534 oct‚ 22 13:51:15 node1 neo-go[1183338]: net/http/server.go:3290 +0x4b4 Signed-off-by: Alexey Savchuk --- cli/server/server.go | 6 +++--- internal/testcli/executor.go | 2 +- pkg/services/rpcsrv/server.go | 4 ++-- pkg/services/rpcsrv/server_helper_test.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cli/server/server.go b/cli/server/server.go index b3f9ec22b..64af4c528 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -505,7 +505,7 @@ func startServer(ctx *cli.Context) error { } errChan := make(chan error) rpcServer := rpcsrv.New(chain, cfg.ApplicationConfiguration.RPC, serv, oracleSrv, log, errChan) - serv.AddService(&rpcServer) + serv.AddService(rpcServer) serv.Start() if !cfg.ApplicationConfiguration.RPC.StartWhenSynchronized { @@ -561,10 +561,10 @@ Main: logLevel.SetLevel(newLogLevel) log.Warn("using new logging level", zap.Stringer("level", newLogLevel)) } - serv.DelService(&rpcServer) + serv.DelService(rpcServer) rpcServer.Shutdown() rpcServer = rpcsrv.New(chain, cfgnew.ApplicationConfiguration.RPC, serv, oracleSrv, log, errChan) - serv.AddService(&rpcServer) + serv.AddService(rpcServer) if !cfgnew.ApplicationConfiguration.RPC.StartWhenSynchronized || serv.IsInSync() { // Here similar to the initial run (see above for-loop), so async. go rpcServer.Start() diff --git a/internal/testcli/executor.go b/internal/testcli/executor.go index 24957db0e..7ec80513c 100644 --- a/internal/testcli/executor.go +++ b/internal/testcli/executor.go @@ -178,7 +178,7 @@ func NewTestChain(t *testing.T, f func(*config.Config), run bool) (*core.Blockch rpcServer := rpcsrv.New(chain, cfg.ApplicationConfiguration.RPC, netSrv, nil, logger, errCh) rpcServer.Start() - return chain, &rpcServer, netSrv + return chain, rpcServer, netSrv } func NewExecutor(t *testing.T, needChain bool) *Executor { diff --git a/pkg/services/rpcsrv/server.go b/pkg/services/rpcsrv/server.go index f25beae04..ecd8b42b7 100644 --- a/pkg/services/rpcsrv/server.go +++ b/pkg/services/rpcsrv/server.go @@ -269,7 +269,7 @@ var rpcWsHandlers = map[string]func(*Server, params.Params, *subscriber) (any, * // New creates a new Server struct. Pay attention that orc is expected to be either // untyped nil or non-nil structure implementing OracleHandler interface. func New(chain Ledger, conf config.RPC, coreServer *network.Server, - orc OracleHandler, log *zap.Logger, errChan chan<- error) Server { + orc OracleHandler, log *zap.Logger, errChan chan<- error) *Server { protoCfg := chain.GetConfig().ProtocolConfiguration if conf.SessionEnabled { if conf.SessionExpirationTime <= 0 { @@ -339,7 +339,7 @@ func New(chain Ledger, conf config.RPC, coreServer *network.Server, } } - return Server{ + return &Server{ http: httpServers, https: tlsServers, diff --git a/pkg/services/rpcsrv/server_helper_test.go b/pkg/services/rpcsrv/server_helper_test.go index 35ef5ae49..97065a13d 100644 --- a/pkg/services/rpcsrv/server_helper_test.go +++ b/pkg/services/rpcsrv/server_helper_test.go @@ -131,7 +131,7 @@ func wrapUnitTestChain(t testing.TB, chain *core.Blockchain, orc OracleHandler, handler := http.HandlerFunc(rpcServer.handleHTTPRequest) srv := httptest.NewServer(handler) t.Cleanup(srv.Close) - return chain, &rpcServer, srv + return chain, rpcServer, srv } func initClearServerWithCustomConfig(t testing.TB, ccfg func(configuration *config.Config)) (*core.Blockchain, *Server, *httptest.Server) {