Skip to content

Commit f70b8dd

Browse files
flound1129claude
andcommitted
fix: address swallowed exceptions and blocking calls
- config.py: log warning instead of silently swallowing callback errors - core.py: move os.statvfs (get_free_space) to thread to avoid blocking the reactor on slow/NFS mounts - common.py: run D-Bus ShowItems in a background thread so a hung file manager doesn't freeze the UI - gtk3/common.py: pickle state in main thread, write+fsync in background thread to avoid blocking GTK on state saves - preferencesmanager.py: add 10s timeout to urlopen in stats sender - tests: fix unused imports flagged by ruff Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5593793 commit f70b8dd

File tree

7 files changed

+53
-34
lines changed

7 files changed

+53
-34
lines changed

deluge/common.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,22 +351,29 @@ def show_file(path, timestamp=None):
351351
timestamp,
352352
)
353353

354-
if dbus:
355-
bus = dbus.SessionBus()
354+
def _show_via_dbus():
356355
try:
356+
bus = dbus.SessionBus()
357357
filemanager1 = bus.get_object(DBUS_FM_ID, DBUS_FM_PATH)
358-
except dbus.exceptions.DBusException as ex:
359-
log.debug('Unable to get dbus file manager: %s', ex)
360-
# Fallback to xdg-open
361-
else:
362358
paths = [urljoin('file:', pathname2url(path))]
363359
filemanager1.ShowItems(paths, startup_id, dbus_interface=DBUS_FM_ID)
364-
return
360+
except Exception as ex:
361+
log.debug('Unable to show file via dbus: %s', ex)
362+
env = os.environ.copy()
363+
env['DESKTOP_STARTUP_ID'] = startup_id.replace('dbus', 'xdg-open')
364+
subprocess.Popen(
365+
['xdg-open', os.path.dirname(path.rstrip('/'))], env=env
366+
)
365367

366-
env = os.environ.copy()
367-
env['DESKTOP_STARTUP_ID'] = startup_id.replace('dbus', 'xdg-open')
368-
# No option in xdg to highlight a file so just open parent folder.
369-
subprocess.Popen(['xdg-open', os.path.dirname(path.rstrip('/'))], env=env)
368+
if dbus:
369+
import threading
370+
371+
threading.Thread(target=_show_via_dbus, daemon=True).start()
372+
else:
373+
env = os.environ.copy()
374+
env['DESKTOP_STARTUP_ID'] = startup_id.replace('dbus', 'xdg-open')
375+
# No option in xdg to highlight a file so just open parent folder.
376+
subprocess.Popen(['xdg-open', os.path.dirname(path.rstrip('/'))], env=env)
370377

371378

372379
def open_url_in_browser(url):

deluge/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ def do_change_callbacks(key, value):
225225

226226
self.callLater(0, do_change_callbacks, key, value)
227227
except Exception:
228-
pass
228+
log.warning('Failed to schedule config change callback for key: %s', key)
229229

230230
# We set the save_timer for 5 seconds if not already set
231231
if not self._save_timer or not self._save_timer.active():

deluge/core/core.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,24 +1276,25 @@ def on_error(failure):
12761276
return d
12771277

12781278
@export
1279-
def get_free_space(self, path: str = None) -> int:
1279+
def get_free_space(self, path: str = None):
12801280
"""Returns the number of free bytes at path
12811281
12821282
Args:
12831283
path: the path to check free space at, if None, use the default download location
12841284
12851285
Returns:
1286-
the number of free bytes at path
1287-
1288-
Raises:
1289-
InvalidPathError: if the path is invalid
1286+
Deferred: the number of free bytes at path, or -1 on error
12901287
"""
12911288
if not path:
12921289
path = self.config['download_location']
1293-
try:
1294-
return deluge.common.free_space(path)
1295-
except InvalidPathError:
1296-
return -1
1290+
1291+
def _get_free_space(path):
1292+
try:
1293+
return deluge.common.free_space(path)
1294+
except InvalidPathError:
1295+
return -1
1296+
1297+
return threads.deferToThread(_get_free_space, path)
12971298

12981299
def _on_external_ip_event(self, external_ip):
12991300
self.external_ip = external_ip

deluge/core/preferencesmanager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ def run(self):
427427
+ '&plugins='
428428
+ quote_plus(':'.join(self.config['enabled_plugins']))
429429
)
430-
urlopen(url)
430+
urlopen(url, timeout=10)
431431
except OSError as ex:
432432
log.debug('Network error while trying to send info: %s', ex)
433433
else:

deluge/tests/test_rpcserver.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from deluge.conftest import BaseTestCase
1313
from deluge.core import rpcserver
1414
from deluge.core.authmanager import AuthManager
15-
from deluge.core.rpcserver import SquallRPCProtocol, RPCServer
15+
from deluge.core.rpcserver import RPCServer, SquallRPCProtocol
1616
from deluge.log import setup_logger
1717

1818
setup_logger('none')

deluge/tests/test_torrent.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import time
99
from base64 import b64encode
1010
from unittest import mock
11+
from unittest.mock import AsyncMock
1112

1213
import pytest
1314
from twisted.internet import defer, reactor
@@ -24,8 +25,6 @@
2425
from deluge.core.torrent import Torrent
2526
from deluge.core.torrentmanager import TorrentManager, TorrentState
2627

27-
from unittest.mock import AsyncMock
28-
2928

3029
class TestTorrent(BaseTestCase):
3130
def setup_config(self):

deluge/ui/gtk3/common.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -284,33 +284,45 @@ def save_pickled_state_file(filename, state):
284284
filename (str): Filename to be saved to config
285285
state (state): The data to be pickled and written to file
286286
"""
287+
import threading
288+
287289
from deluge.configmanager import get_config_dir
288290

289291
filepath = os.path.join(get_config_dir(), 'gtk3ui_state', filename)
290292
filepath_bak = filepath + '.bak'
291293
filepath_tmp = filepath + '.tmp'
292294

295+
# Pickle in the main thread (not thread-safe), write in a background thread.
293296
try:
294-
if os.path.isfile(filepath):
295-
log.debug('Creating backup of %s at: %s', filename, filepath_bak)
296-
shutil.copy2(filepath, filepath_bak)
297-
except OSError as ex:
298-
log.error('Unable to backup %s to %s: %s', filepath, filepath_bak, ex)
299-
else:
297+
data = pickle.dumps(state, protocol=2)
298+
except pickle.PicklingError as ex:
299+
log.error('Unable to pickle %s: %s', filename, ex)
300+
return
301+
302+
def _write():
303+
try:
304+
if os.path.isfile(filepath):
305+
log.debug('Creating backup of %s at: %s', filename, filepath_bak)
306+
shutil.copy2(filepath, filepath_bak)
307+
except OSError as ex:
308+
log.error('Unable to backup %s to %s: %s', filepath, filepath_bak, ex)
309+
return
310+
300311
log.info('Saving the %s at: %s', filename, filepath)
301312
try:
302313
with open(filepath_tmp, 'wb') as _file:
303-
# Pickle the state object
304-
pickle.dump(state, _file, protocol=2)
314+
_file.write(data)
305315
_file.flush()
306316
os.fsync(_file.fileno())
307317
shutil.move(filepath_tmp, filepath)
308-
except (OSError, EOFError, pickle.PicklingError) as ex:
318+
except (OSError, EOFError) as ex:
309319
log.error('Unable to save %s: %s', filename, ex)
310320
if os.path.isfile(filepath_bak):
311321
log.info('Restoring backup of %s from: %s', filename, filepath_bak)
312322
shutil.move(filepath_bak, filepath)
313323

324+
threading.Thread(target=_write, daemon=True).start()
325+
314326

315327
def load_pickled_state_file(filename):
316328
"""Loads a file from the config directory, attempting backup if original fails to load.

0 commit comments

Comments
 (0)