Skip to content

Commit 1867b22

Browse files
committed
added fixes for automatically registering signal handler on non-main thread
1 parent 23f2552 commit 1867b22

File tree

4 files changed

+93
-11
lines changed

4 files changed

+93
-11
lines changed

ibind/base/rest_client.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def __init__(
8484
timeout: float = 10,
8585
max_retries: int = 3,
8686
use_session: bool = var.IBIND_USE_SESSION,
87+
auto_register_shutdown: bool = var.IBIND_AUTO_REGISTER_SHUTDOWN,
8788
) -> None:
8889
"""
8990
Parameters:
@@ -93,6 +94,7 @@ def __init__(
9394
timeout (float, optional): Timeout in seconds for the API requests. Defaults to 10.
9495
max_retries (int, optional): Maximum number of retries for failed API requests. Defaults to 3.
9596
use_session (bool, optional): Whether to use a persistent session for making requests. Defaults to True.
97+
auto_register_shutdown (bool, optional): Whether to automatically register a shutdown handler for this client. Defaults to True.
9698
"""
9799

98100
if url is None:
@@ -115,17 +117,15 @@ def __init__(
115117
if use_session:
116118
self.make_session()
117119

118-
self.register_shutdown_handler()
120+
if auto_register_shutdown:
121+
self.register_shutdown_handler()
119122

120123
def _make_logger(self):
121124
self._logger = new_daily_rotating_file_handler('RestClient', os.path.join(var.LOGS_DIR, f'rest_client'))
122125

123126
def make_session(self):
124127
"""Creates a new session, ensuring old one (if exists) is closed properly."""
125128
self._session = requests.Session()
126-
adapter = HTTPAdapter(pool_connections=10, pool_maxsize=10, pool_block=True)
127-
self._session.mount("https://", adapter)
128-
self._session.mount("http://", adapter)
129129

130130
@property
131131
def logger(self):
@@ -306,7 +306,6 @@ def _close_handler():
306306
if self._closed:
307307
return
308308
self._closed = True
309-
310309
self.close()
311310

312311
def _signal_handler(signum, frame):
@@ -318,8 +317,14 @@ def _signal_handler(signum, frame):
318317
if signum == signal.SIGTERM and callable(existing_handler_term):
319318
existing_handler_term(signum, frame)
320319

321-
signal.signal(signal.SIGINT, _signal_handler)
322-
signal.signal(signal.SIGTERM, _signal_handler)
320+
try:
321+
signal.signal(signal.SIGINT, _signal_handler)
322+
signal.signal(signal.SIGTERM, _signal_handler)
323+
except ValueError as e:
324+
if str(e) == 'signal only works in main thread of the main interpreter':
325+
pass # we cannot register signal, we ignore it and continue working as normal
326+
else:
327+
raise
323328
atexit.register(_close_handler)
324329

325330
def __str__(self):

ibind/client/ibkr_client.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ def __init__(
5050
timeout: float = 10,
5151
max_retries: int = 3,
5252
use_session: bool = var.IBIND_USE_SESSION,
53+
auto_register_shutdown: bool = var.IBIND_AUTO_REGISTER_SHUTDOWN,
5354
use_oauth: bool = var.IBIND_USE_OAUTH,
54-
oauth_config: 'OAuthConfig' = None
55+
oauth_config: 'OAuthConfig' = None,
56+
5557
) -> None:
5658
"""
5759
Parameters:
@@ -69,6 +71,7 @@ def __init__(
6971
timeout (float, optional): Timeout in seconds for the API requests. Defaults to 10.
7072
max_retries (int, optional): Maximum number of retries for failed API requests. Defaults to 3.
7173
use_session (bool, optional): Whether to use a persistent session for making requests. Defaults to True.
74+
auto_register_shutdown (bool, optional): Whether to automatically register a shutdown handler for this client. Defaults to True.
7275
use_oauth (bool, optional): Whether to use OAuth authentication. Defaults to False.
7376
oauth_config (OAuthConfig, optional): The configuration for the OAuth authentication. OAuth1aConfig is used if not specified.
7477
"""
@@ -87,7 +90,14 @@ def __init__(
8790
self.account_id = account_id
8891

8992
cacert = True if self._use_oauth else cacert
90-
super().__init__(url=url, cacert=cacert, timeout=timeout, max_retries=max_retries, use_session=use_session)
93+
super().__init__(
94+
url=url,
95+
cacert=cacert,
96+
timeout=timeout,
97+
max_retries=max_retries,
98+
use_session=use_session,
99+
auto_register_shutdown=auto_register_shutdown,
100+
)
91101

92102
self.logger.info('#################')
93103
self.logger.info(f'New IbkrClient(base_url={self.base_url!r}, account_id={self.account_id!r}, ssl={self.cacert!r}, timeout={self._timeout}, max_retries={self._max_retries}, use_oauth={self._use_oauth})')
@@ -248,4 +258,4 @@ def oauth_shutdown(self):
248258
"""
249259
_LOGGER.info(f'{self}: Shutting down OAuth')
250260
self.stop_tickler()
251-
self.logout()
261+
self.logout()

ibind/var.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ def to_bool(value):
3333
IBIND_USE_SESSION = to_bool(os.environ.get('IBIND_USE_SESSION', True))
3434
""" Whether to use persistent session in REST requests. """
3535

36+
IBIND_AUTO_REGISTER_SHUTDOWN = to_bool(os.environ.get('IBIND_AUTO_REGISTER_SHUTDOWN', True))
37+
""" Whether to automatically register the shutdown handler. """
38+
3639
##### LOGS #####
3740

3841
LOG_TO_CONSOLE = to_bool(os.environ.get('IBIND_LOG_TO_CONSOLE', True))

test/integration/base/test_rest_client_i.py

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
import threading
12
from unittest import TestCase
23
from unittest.mock import patch, MagicMock
4+
import asyncio
35

46
from requests import ReadTimeout, Timeout
57

@@ -10,7 +12,7 @@
1012

1113

1214
@patch('ibind.base.rest_client.requests')
13-
class TestIbkrClientI(TestCase):
15+
class TestRestClientI(TestCase):
1416
def setUp(self):
1517
self.url = 'https://localhost:5000'
1618
self.account_id = 'TEST_ACCOUNT_ID'
@@ -81,3 +83,65 @@ def test_response_raise_generic(self, requests_mock):
8183
self.client.get(self.default_path)
8284

8385
self.assertEqual(f"RestClient: response error {self.result.copy(data=None)} :: {self.response.status_code} :: {self.response.reason} :: {self.response.text}", str(cm_err.exception))
86+
87+
class TestRestClientInThread(TestCase):
88+
def _worker(self, results:[]):
89+
try:
90+
IbkrClient()
91+
except Exception as e:
92+
results.append(e)
93+
94+
def test_in_thread(self):
95+
""" Run in thread ensuring client still is constructed without an exception."""
96+
results = []
97+
t = threading.Thread(target=self._worker, args=(results,))
98+
t.daemon = True
99+
t.start()
100+
t.join(1)
101+
for result in results:
102+
if isinstance(result, Exception):
103+
raise result
104+
105+
106+
def test_without_thread(self):
107+
""" Run without a thread to ensure it still works as expected."""
108+
results = []
109+
self._worker(results)
110+
for result in results:
111+
if isinstance(result, Exception):
112+
raise result
113+
114+
115+
class TestRestClientAsync(TestCase):
116+
def _worker(self, results: []):
117+
"""Runs the async test inside a new thread to check if signal handling breaks."""
118+
try:
119+
asyncio.run(self._async_worker(results))
120+
except Exception as e:
121+
results.append(e)
122+
123+
async def _async_worker(self, results: []):
124+
"""Async version of the worker function to run in an asyncio event loop."""
125+
try:
126+
IbkrClient()
127+
except Exception as e:
128+
results.append(e)
129+
130+
def test_in_thread_async(self):
131+
"""Test that IbkrClient() does not break in an asyncio thread."""
132+
results = []
133+
t = threading.Thread(target=self._worker, args=(results,))
134+
t.daemon = True
135+
t.start()
136+
t.join(1)
137+
for result in results:
138+
if isinstance(result, Exception):
139+
raise result
140+
141+
def test_without_thread_async(self):
142+
"""Test that IbkrClient() does not break in the main asyncio event loop."""
143+
results = []
144+
asyncio.run(self._async_worker(results))
145+
for result in results:
146+
if isinstance(result, Exception):
147+
raise result

0 commit comments

Comments
 (0)