From 9d47a97b0b79a40b22ce62a724d5aed54b50356d Mon Sep 17 00:00:00 2001
From: Marek Siarkowicz <siarkowicz@google.com>
Date: Fri, 15 Oct 2021 16:24:47 +0200
Subject: [PATCH] server: Remove lock from adapter to avoid deadlock

---
 server/etcdserver/adapters.go             | 31 ++++++-----------------
 server/etcdserver/version/monitor.go      |  5 ----
 server/etcdserver/version/monitor_test.go | 11 --------
 server/etcdserver/version/version_test.go |  6 -----
 4 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/server/etcdserver/adapters.go b/server/etcdserver/adapters.go
index 1def9b69e..bc4b68645 100644
--- a/server/etcdserver/adapters.go
+++ b/server/etcdserver/adapters.go
@@ -23,20 +23,17 @@ import (
 	"go.etcd.io/etcd/api/v3/membershippb"
 	"go.etcd.io/etcd/api/v3/version"
 	serverversion "go.etcd.io/etcd/server/v3/etcdserver/version"
-	"go.etcd.io/etcd/server/v3/storage/backend"
 	"go.etcd.io/etcd/server/v3/storage/schema"
 )
 
 // serverVersionAdapter implements Server interface needed by serverversion.Monitor
 type serverVersionAdapter struct {
 	*EtcdServer
-	tx backend.BatchTx
 }
 
 func newServerVersionAdapter(s *EtcdServer) *serverVersionAdapter {
 	return &serverVersionAdapter{
 		EtcdServer: s,
-		tx:         nil,
 	}
 }
 
@@ -75,11 +72,10 @@ func (s *serverVersionAdapter) GetMembersVersions() map[string]*version.Versions
 }
 
 func (s *serverVersionAdapter) GetStorageVersion() *semver.Version {
-	if s.tx == nil {
-		s.Lock()
-		defer s.Unlock()
-	}
-	v, err := schema.UnsafeDetectSchemaVersion(s.lg, s.tx)
+	tx := s.be.BatchTx()
+	tx.Lock()
+	defer tx.Unlock()
+	v, err := schema.UnsafeDetectSchemaVersion(s.lg, tx)
 	if err != nil {
 		return nil
 	}
@@ -87,19 +83,8 @@ func (s *serverVersionAdapter) GetStorageVersion() *semver.Version {
 }
 
 func (s *serverVersionAdapter) UpdateStorageVersion(target semver.Version) error {
-	if s.tx == nil {
-		s.Lock()
-		defer s.Unlock()
-	}
-	return schema.UnsafeMigrate(s.lg, s.tx, s.r.storage, target)
-}
-
-func (s *serverVersionAdapter) Lock() {
-	s.tx = s.be.BatchTx()
-	s.tx.Lock()
-}
-
-func (s *serverVersionAdapter) Unlock() {
-	s.tx.Unlock()
-	s.tx = nil
+	tx := s.be.BatchTx()
+	tx.Lock()
+	defer tx.Unlock()
+	return schema.UnsafeMigrate(s.lg, tx, s.r.storage, target)
 }
diff --git a/server/etcdserver/version/monitor.go b/server/etcdserver/version/monitor.go
index 05da0b228..8ac8d8e8d 100644
--- a/server/etcdserver/version/monitor.go
+++ b/server/etcdserver/version/monitor.go
@@ -40,9 +40,6 @@ type Server interface {
 
 	GetStorageVersion() *semver.Version
 	UpdateStorageVersion(semver.Version) error
-
-	Lock()
-	Unlock()
 }
 
 func NewMonitor(lg *zap.Logger, storage Server) *Monitor {
@@ -100,8 +97,6 @@ func (m *Monitor) UpdateStorageVersionIfNeeded() {
 	if cv == nil {
 		return
 	}
-	m.s.Lock()
-	defer m.s.Unlock()
 	sv := m.s.GetStorageVersion()
 
 	if sv == nil || sv.Major != cv.Major || sv.Minor != cv.Minor {
diff --git a/server/etcdserver/version/monitor_test.go b/server/etcdserver/version/monitor_test.go
index a6b31cec8..b11f09581 100644
--- a/server/etcdserver/version/monitor_test.go
+++ b/server/etcdserver/version/monitor_test.go
@@ -421,14 +421,3 @@ func (s *storageMock) UpdateStorageVersion(v semver.Version) error {
 	s.storageVersion = &v
 	return nil
 }
-
-func (s *storageMock) Lock() {
-	if s.locked {
-		panic("Deadlock")
-	}
-	s.locked = true
-}
-
-func (s *storageMock) Unlock() {
-	s.locked = false
-}
diff --git a/server/etcdserver/version/version_test.go b/server/etcdserver/version/version_test.go
index 70578ad6b..c82ca808c 100644
--- a/server/etcdserver/version/version_test.go
+++ b/server/etcdserver/version/version_test.go
@@ -256,9 +256,3 @@ func (m *memberMock) UpdateStorageVersion(v semver.Version) error {
 
 func (m *memberMock) TriggerSnapshot() {
 }
-
-func (m *memberMock) Lock() {
-}
-
-func (m *memberMock) Unlock() {
-}