jasonrbriggs / stomp.py

“stomp.py” is a Python client library for accessing messaging servers (such as ActiveMQ or RabbitMQ) using the STOMP protocol (versions 1.0, 1.1 and 1.2). It can also be run as a standalone, command-line client for testing.
Apache License 2.0
491 stars 167 forks source link

test_heartbeat_timeout.py: reproducer for thread-safety issues in HeartbeatListener #390

Open mikebonnet opened 2 years ago

mikebonnet commented 2 years ago

Python sockets are not thread-safe. These tests expose an issue where, in the event of a heartbeat timeout, the heartbeat thread closes the socket while the receiver thread created by the transport is reading from it. This can lead to memory corruption and segmentation faults. Related to #389 .

mikebonnet commented 2 years ago

Example output I get when running these tests locally:

DEBUG    stomp.py:transport.py:191 Received frame: 'heartbeat', headers={}, body=None
INFO     stomp.py:listener.py:466 on_heartbeat
INFO     stomp.py:test_heartbeat_timeout.py:22 Cutting receive_sleep in half
WARNING  stomp.py:listener.py:304 Heartbeat timeout: diff_receive=0.07520760386250913, time=1489045.61509271, lastrec=1489045.539885106
DEBUG    stomp.py:transport.py:390 nothing received, raising CCE
Fatal Python error: Aborted

Current thread 0x00007f69b6cbb640 (most recent call first):
  File "/usr/lib64/python3.10/ssl.py", line 1317 in unwrap
  File "/home/mikeb/opt/github/stomp.py/stomp/transport.py", line 591 in disconnect_socket
  File "/home/mikeb/opt/github/stomp.py/stomp/listener.py", line 307 in __heartbeat_loop
  File "/usr/lib64/python3.10/threading.py", line 946 in run
  File "/usr/lib64/python3.10/threading.py", line 1009 in _bootstrap_inner
  File "/usr/lib64/python3.10/threading.py", line 966 in _bootstrap

Thread 0x00007f69b632b640 (most recent call first):
  File "/usr/lib64/python3.10/socket.py", line 496 in _real_close
  File "/usr/lib64/python3.10/ssl.py", line 1332 in _real_close
  File "/usr/lib64/python3.10/socket.py", line 502 in close
  File "/home/mikeb/opt/github/stomp.py/stomp/transport.py", line 655 in cleanup
  File "/home/mikeb/opt/github/stomp.py/stomp/transport.py", line 358 in __receiver_loop
  File "/usr/lib64/python3.10/threading.py", line 946 in run
  File "/usr/lib64/python3.10/threading.py", line 1009 in _bootstrap_inner
  File "/usr/lib64/python3.10/threading.py", line 966 in _bootstrap

Thread 0x00007f69c5f57740 (most recent call first):
  File "/usr/lib64/python3.10/threading.py", line 320 in wait
  File "/home/mikeb/opt/github/stomp.py/stomp/listener.py", line 366 in wait_on_disconnected
  File "/home/mikeb/opt/github/stomp.py/tests/test_heartbeat_timeout.py", line 51 in test_heartbeat_timeout_reconnect
jasonrbriggs commented 2 years ago

FYI, local testing doesn't work for me with this one. Goes into a loop and then eventually dumps core.

mikebonnet commented 2 years ago

That’s the point actually. :) The HeartbeatListener closes the socket from outside the main thread, which leads to memory corruption and core dumps. This test is exposing the problem, I don’t yet have a good solution. No need to merge this yet, I just wanted to call attention to it. Open to suggestions for a fix.

jasonrbriggs commented 2 years ago

That’s the point actually. :)

LOL, yeah sorry I suddenly realised that after I commented and then went to bed. It was a late night senile moment...

I'll give it some further as well. Not sure how to resolve either...

s17b2-voroneckij commented 1 year ago

@jasonrbriggs, are there any plans for fixing this? Or at least a way to overcome this issue?

jasonrbriggs commented 1 year ago

No plans at the moment. I started working on a rewrite using asyncio (https://github.com/jasonrbriggs/stomp.py/tree/asyncio-experimental), which might have been a solution, but never got it working completely, nor had the time to finish it.