Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

self._call_lock never released when trying to send payload on a closed socket #46

Closed
psauer opened this issue Nov 15, 2019 · 1 comment · Fixed by #47
Closed

self._call_lock never released when trying to send payload on a closed socket #46

psauer opened this issue Nov 15, 2019 · 1 comment · Fixed by #47
Labels
bug Something isn't working

Comments

@psauer
Copy link

psauer commented Nov 15, 2019

ocpp/ocpp/charge_point.py

Line 251 in 75dd91f

await self._send(call.to_json())

If you try and send a payload, but the socket connection is terminated by the server just before trying to send, a websockets.exceptions.ConnectionClosedError is raised but never caught here. This leads to self._call_lock.release() never being called, making it impossible to send data again.
@OrangeTux
Copy link
Collaborator

Good catch! Thanks for reporting it

@OrangeTux OrangeTux added the bug Something isn't working label Nov 15, 2019
OrangeTux pushed a commit that referenced this issue Nov 15, 2019
Fix potential deadlock
7cb1b31 
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
@OrangeTux OrangeTux mentioned this issue Nov 15, 2019
Merged
proelke pushed a commit that referenced this issue Nov 18, 2019
@proelke
Fix potential deadlock
ea46971 
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
@OrangeTux OrangeTux mentioned this issue Nov 18, 2019
Closed
OrangeTux pushed a commit that referenced this issue Nov 18, 2019
Don't reraise exception in finally block
90c2600 
The fix introduced for #46 contained a bug. `ChargePoint.call()`
could fail with:

	try:
	    await self._send(call.to_json())
	    response = \
		await self._get_specific_response(call.unique_id,
						  self._response_timeout)
	finally:
	    self._call_lock.release()
>           raise
E           RuntimeError: No active exception to reraise
OrangeTux pushed a commit that referenced this issue Nov 18, 2019
Don't reraise exception in finally block
0cb645c 
The fix introduced for #46 contained a bug. `ChargePoint.call()`
could fail with:

	try:
	    await self._send(call.to_json())
	    response = \
		await self._get_specific_response(call.unique_id,
						  self._response_timeout)
	finally:
	    self._call_lock.release()
>           raise
E           RuntimeError: No active exception to reraise

Fixes: #50
@OrangeTux OrangeTux mentioned this issue Nov 18, 2019
Merged
OrangeTux added a commit that referenced this issue Nov 18, 2019
@OrangeTux
Don't reraise exception in finally block
34f94bd 
The fix introduced for #46 contained a bug. `ChargePoint.call()`
could fail with:

	try:
	    await self._send(call.to_json())
	    response = \
		await self._get_specific_response(call.unique_id,
						  self._response_timeout)
	finally:
	    self._call_lock.release()
>           raise
E           RuntimeError: No active exception to reraise

Fixes: #50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix potential deadlock mobilityhouse/ocpp
2 participants
@OrangeTux @psauer