Skip to content

Commit

Permalink
CA-400275 Tolerate a restarting fairlock service while connecting
Browse files Browse the repository at this point in the history
Cope with the case where the fairlock service is restarted while
something is in the middle of connecting to it.

Signed-off-by: Tim Smith <tim.smith@cloud.com>
  • Loading branch information
Tim Smith committed Oct 15, 2024
1 parent d138bd1 commit 748c022
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*.o
*.swp
*.pyc
*.kate-swp
tests/faultinjection/pfilter
tests/faultinjection/libipq.c
tests/faultinjection/libipq/libipq.h
Expand Down
20 changes: 14 additions & 6 deletions misc/fairlock/fairlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,29 @@ def _ensure_service(self):
if time.time() > timeout:
raise FairlockServiceTimeout(f"Timed out starting service {service}")

def _connect_and_recv(self):
while True:
self.sock.connect(self.sockname)
# Merely being connected is not enough. Read a small blob of data.
b = self.sock.recv(10)
if len(b) > 0:
return True
# If we got a zero-length return, it means the service exited while we
# were waiting. Any timeout we put here would be a max wait time to acquire
# the lock, which is dangerous.
self._ensure_service()

def __enter__(self):
if self.connected:
raise FairlockDeadlock(f"Deadlock on Fairlock resource '{self.name}'")

self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
self.sock.setblocking(True)
try:
self.sock.connect(self.sockname)
# Merely being connected is not enough. Read a small blob of data.
self.sock.recv(10)
self._connect_and_recv()
except (FileNotFoundError, ConnectionRefusedError):
self._ensure_service()
self.sock.connect(self.sockname)
# Merely being connected is not enough. Read a small blob of data.
self.sock.recv(10)
self._connect_and_recv()

self.sock.send(f'{os.getpid()} - {time.monotonic()}'.encode())
self.connected = True
Expand Down
10 changes: 8 additions & 2 deletions tests/test_fairlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def test_first_lock(self):
mock_sock = mock.MagicMock()
self.mock_socket.socket.return_value = mock_sock
mock_sock.connect.side_effect = [FileNotFoundError(), 0]
mock_sock.recv.side_effect = [b'Foop']
self.mock_os.system.side_effect = [0, 1, 0]
self.mock_time.time.side_effect = [0, 0, 0]

Expand All @@ -38,6 +39,7 @@ def test_first_lock_timeout(self):
mock_sock = mock.MagicMock()
self.mock_socket.socket.return_value = mock_sock
mock_sock.connect.side_effect = [FileNotFoundError(), 0]
mock_sock.recv.side_effect = [b'Foop']
self.mock_os.system.side_effect = [0, 1, 1, 1, 0]
self.mock_time.time.side_effect = [0, 1, 3]

Expand All @@ -53,6 +55,7 @@ def test_second_lock(self):
mock_sock = mock.MagicMock()
self.mock_socket.socket.return_value = mock_sock
mock_sock.connect.side_effect = [0]
mock_sock.recv.side_effect = [b'Foop']

with Fairlock("test"):
print("Hello World")
Expand All @@ -68,6 +71,8 @@ def test_two_locks(self):
self.mock_socket.socket.side_effect = [mock_sock1, mock_sock2]
mock_sock1.connect.side_effect = [FileNotFoundError(), 0]
mock_sock2.connect.side_effect = [FileNotFoundError(), 0]
mock_sock1.recv.side_effect = [b'Foop']
mock_sock2.recv.side_effect = [b'Foop']
self.mock_os.system.side_effect = [0, 1, 0, 0, 1, 0]
self.mock_time.time.side_effect = [0, 0, 0, 0, 0, 0]

Expand All @@ -81,15 +86,16 @@ def test_double_lock_deadlock(self):
Test double usage of the same lock
"""
mock_sock = mock.MagicMock()
self.mock_socket.socket.side_effect = [mock_sock]
self.mock_socket.socket.side_effect = [mock_sock]
mock_sock.connect.side_effect = [FileNotFoundError(), 0]
mock_sock.recv.side_effect = [b'Foop', b'Foop']
self.mock_os.system.side_effect = [0, 1, 0, 0, 1, 0]
self.mock_time.time.side_effect = [0, 0, 0, 0, 0, 0]

with self.assertRaises(FairlockDeadlock) as err:
with Fairlock("test") as l:
n = Fairlock("test")
self.assertEquals(l, n)
self.assertEqual(l, n)
# Real code would use another 'with Fairlock("test")' here but we cannot
# do that because it insists on having a code block as a body, which would
# then not be reached, causing a "Test code not fully covered" failure
Expand Down

0 comments on commit 748c022

Please sign in to comment.