Skip to content

Commit

Permalink
Fix potential deadlock
Browse files Browse the repository at this point in the history
ocpp.ChargePoint.call() implements flow control as defined in the OCPP
specification. It makes sure that no call can be send after an answer on
the previous request has been received.

A lock is used to implement flow control. If the call `await
self._send(call.json())` crashed it the lock wouldn't be released
which could lead to a deadlock.

Fixes: #46
  • Loading branch information
Auke Willem Oosterhoff committed Nov 15, 2019
1 parent 75dd91f commit 7cb1b31
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
6 changes: 2 additions & 4 deletions ocpp/charge_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,16 @@ async def call(self, payload):
# Use a lock to prevent make sure that only 1 message can be send at a
# a time.
await self._call_lock.acquire()
await self._send(call.to_json())

try:
await self._send(call.to_json())
response = \
await self._get_specific_response(call.unique_id,
self._response_timeout)
except asyncio.TimeoutError:
finally:
self._call_lock.release()
raise

self._call_lock.release()

if response.message_type_id == MessageType.CallError:
LOGGER.warning("Received a CALLError: %s'", response)
return
Expand Down
22 changes: 21 additions & 1 deletion tests/v16/test_v16_charge_point.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import json
import pytest
import asyncio

from ocpp.exceptions import NotImplementedError
from ocpp.routing import on, after, create_route_map
from ocpp.v16.enums import Action
from ocpp.v16 import call_result
from ocpp.v16 import call_result, call, ChargePoint


@pytest.mark.asyncio
Expand Down Expand Up @@ -64,3 +65,22 @@ async def test_route_message_with_no_route(base_central_system,

with pytest.raises(NotImplementedError):
await base_central_system.route_message(heartbeat_call)


@pytest.mark.asyncio
async def test_send_call_with_timeout(connection):
cs = ChargePoint(
id=1234,
connection=connection,
response_timeout=0.1
)

payload = call.ResetPayload(type="Hard")

with pytest.raises(asyncio.TimeoutError):
await cs.call(payload)

# Verify that lock is released if call() crahses. Not releasing the lock
# in case of an exception could lead to a deadlock. See
# https://github.com/mobilityhouse/ocpp/issues/46
assert cs._call_lock.locked() is False

0 comments on commit 7cb1b31

Please sign in to comment.