Ocpp: fix deadlock when client stops transaction (evcc-io#10355)

honeytreelabs · Oct 16, 2023 · 2fb9c7d · 2fb9c7d
1 parent 7c18561
commit 2fb9c7d
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 14 deletions.
diff --git a/charger/ocpp.go b/charger/ocpp.go
@@ -274,6 +274,11 @@ func NewOCPP(id string, connector int, idtag string,
 	return c, conn.Initialized()
 }
 
+// Connector returns the connector instance
+func (c *OCPP) Connector() *ocpp.Connector {
+	return c.conn
+}
+
 // hasMeasurement checks if meterValuesSample contains given measurement
 func (c *OCPP) hasMeasurement(val types.Measurand) bool {
 	return slices.Contains(strings.Split(c.meterValuesSample, ","), string(val))

diff --git a/charger/ocpp/cp.go b/charger/ocpp/cp.go
@@ -11,7 +11,7 @@ import (
 // Since ocpp-go interfaces at charge point level, we need to manage multiple connector separately
 
 type CP struct {
-	mu   sync.Mutex
+	mu   sync.RWMutex
 	once sync.Once
 	log  *util.Logger
 
@@ -46,15 +46,15 @@ func (cp *CP) registerConnector(id int, conn *Connector) error {
 }
 
 func (cp *CP) connectorByID(id int) *Connector {
-	cp.mu.Lock()
-	defer cp.mu.Unlock()
+	cp.mu.RLock()
+	defer cp.mu.RUnlock()
 
 	return cp.connectors[id]
 }
 
 func (cp *CP) connectorByTransactionID(id int) *Connector {
-	cp.mu.Lock()
-	defer cp.mu.Unlock()
+	cp.mu.RLock()
+	defer cp.mu.RUnlock()
 
 	for _, conn := range cp.connectors {
 		if txn, err := conn.TransactionID(); err == nil && txn == id {
@@ -66,8 +66,8 @@ func (cp *CP) connectorByTransactionID(id int) *Connector {
 }
 
 func (cp *CP) ID() string {
-	cp.mu.Lock()
-	defer cp.mu.Unlock()
+	cp.mu.RLock()
+	defer cp.mu.RUnlock()
 
 	return cp.id
 }
@@ -97,8 +97,8 @@ func (cp *CP) connect(connect bool) {
 }
 
 func (cp *CP) Connected() bool {
-	cp.mu.Lock()
-	defer cp.mu.Unlock()
+	cp.mu.RLock()
+	defer cp.mu.RUnlock()
 
 	return cp.connected
 }

diff --git a/charger/ocpp_test.go b/charger/ocpp_test.go
@@ -95,13 +95,14 @@ func (suite *ocppTestSuite) handleTrigger(cp ocpp16.ChargePoint, connectorId int
 func (suite *ocppTestSuite) TestConnect() {
 	// 1st charge point- remote
 	cp1 := suite.startChargePoint("test-1", 1)
-	suite.NoError(cp1.Start(ocppTestUrl))
-	suite.True(cp1.IsConnected())
+	suite.Require().NoError(cp1.Start(ocppTestUrl))
+	suite.Require().True(cp1.IsConnected())
 
 	// 1st charge point- local
 	c1, err := NewOCPP("test-1", 1, "", "", 0, false, false, ocppTestConnectTimeout, ocppTestTimeout, "A")
 	suite.Require().NoError(err)
 
+	// status and meter values
 	{
 		suite.clock.Add(ocppTestTimeout)
 		c1.conn.TestClock(suite.clock)
@@ -121,6 +122,35 @@ func (suite *ocppTestSuite) TestConnect() {
 		suite.Equal(1.2, f)
 	}
 
+	// takeover
+	{
+		expectedTxn := 99
+
+		_, err := cp1.StopTransaction(0, types.NewDateTime(suite.clock.Now()), expectedTxn)
+		suite.Require().Error(err)
+
+		_, err = cp1.MeterValues(1, []types.MeterValue{
+			{
+				Timestamp: types.NewDateTime(suite.clock.Now()),
+				SampledValue: []types.SampledValue{
+					{Measurand: types.MeasurandPowerActiveImport, Value: "1000"},
+				},
+			},
+		}, func(request *core.MeterValuesRequest) {
+			request.TransactionId = &expectedTxn
+		})
+		suite.Require().NoError(err)
+
+		conn1 := c1.Connector()
+		txnId, err := conn1.TransactionID()
+		suite.Require().NoError(err)
+		suite.Equal(expectedTxn, txnId)
+
+		res, err := cp1.StopTransaction(0, types.NewDateTime(suite.clock.Now()), expectedTxn)
+		suite.Require().NoError(err)
+		suite.Equal(res.IdTagInfo.Status, types.AuthorizationStatusAccepted)
+	}
+
 	// 2nd charge point - remote
 	cp2 := suite.startChargePoint("test-2", 1)
 	suite.Require().NoError(cp2.Start(ocppTestUrl))

diff --git a/go.mod b/go.mod
@@ -59,7 +59,7 @@ require (
 	github.com/korylprince/ipnetgen v1.0.1
 	github.com/kr/pretty v0.3.1
 	github.com/libp2p/zeroconf/v2 v2.2.0
-	github.com/lorenzodonini/ocpp-go v0.17.0
+	github.com/lorenzodonini/ocpp-go v0.17.1-0.20231009195058-5be50a50e6e9
 	github.com/lunixbochs/struc v0.0.0-20200707160740-784aaebc1d40
 	github.com/mabunixda/wattpilot v1.6.2
 	github.com/manifoldco/promptui v0.9.0

diff --git a/go.sum b/go.sum
@@ -527,8 +527,8 @@ github.com/libp2p/zeroconf/v2 v2.2.0 h1:Cup06Jv6u81HLhIj1KasuNM/RHHrJ8T7wOTS4+Tv
 github.com/libp2p/zeroconf/v2 v2.2.0/go.mod h1:fuJqLnUwZTshS3U/bMRJ3+ow/v9oid1n0DmyYyNO1Xs=
 github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM=
 github.com/lightstep/lightstep-tracer-go v0.18.1/go.mod h1:jlF1pusYV4pidLvZ+XD0UBX0ZE6WURAspgAczcDHrL4=
-github.com/lorenzodonini/ocpp-go v0.17.0 h1:dDukZNxtQhlDfai20P/jFbJCxinnqx44ml/mrcawfGQ=
-github.com/lorenzodonini/ocpp-go v0.17.0/go.mod h1:jVzgk5k3JVGltWMU/QGlU7chojlCd/bJYc4WYl8R2M4=
+github.com/lorenzodonini/ocpp-go v0.17.1-0.20231009195058-5be50a50e6e9 h1:vyxXHKvBUmBPpiFwcUEDDLo8pMpe0P1G7mnmLd202Ck=
+github.com/lorenzodonini/ocpp-go v0.17.1-0.20231009195058-5be50a50e6e9/go.mod h1:jVzgk5k3JVGltWMU/QGlU7chojlCd/bJYc4WYl8R2M4=
 github.com/lunixbochs/struc v0.0.0-20200707160740-784aaebc1d40 h1:EnfXoSqDfSNJv0VBNqY/88RNnhSGYkrHaO0mmFGbVsc=
 github.com/lunixbochs/struc v0.0.0-20200707160740-784aaebc1d40/go.mod h1:vy1vK6wD6j7xX6O6hXe621WabdtNkou2h7uRtTfRMyg=
 github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0QoUACkjt2znoq26NVQ=