Skip to content

fix: memory and semaphore leaks#591

Draft
thismanyboyfriends2 wants to merge 5 commits intodatawhores:mainfrom
thismanyboyfriends2:fix/memory-leaks
Draft

fix: memory and semaphore leaks#591
thismanyboyfriends2 wants to merge 5 commits intodatawhores:mainfrom
thismanyboyfriends2:fix/memory-leaks

Conversation

@thismanyboyfriends2
Copy link
Copy Markdown

I've noticed from using OFscraper a fair bit that it doesn't seem to handle force closes or ctrl-c particularly well, especially on retries. Sometimes when running ofscraper a few times concurrently it seems to totally freeze up, even struggling to return the --help dialog. I've often had to resort to restarting the OS in order to free up resources.

This PR is an attempt to clear up some of those issues:

  • Semaphore deadlock (main_download.py, alt_download.py) — Early returns didn't release semaphores; moved acquire/release to try/finally at method entry
  • Manager reuse bug (manager.py) — manager is None was comparison, not assignment; fixed to manager = None
  • ThreadPoolExecutor never shutdown (exit.py) — Added closeThreadExecutor() with wait=True to exit handlers
  • Daemon thread infinite loop (scraper.py) — Changed daemon=True to daemon=False, added join(timeout=5) in exception handlers
  • Event loop cleanup failures (run_async.py) — Replaced bare except: with specific exception handlers; explicit RuntimeError catch for already-closed loops
  • Database pool not shutdown on success (wrapper.py) — Added finally block with shutdown(wait=True) on all paths
  • Semaphore double-release risk (sessionmanager.py) — Consolidated manual releases into single finally block
  • Asyncio task exceptions uncaught (download.py, metadata.py) — Added exception callbacks and return_exceptions=True to gather calls

I've been running it against a few profiles locally and this appears to work much better now, but I'm aware its a lot of changes - and there aren't many tests I can verify these fixes with. Any feedback is welcome.

Thanks!

@thismanyboyfriends2 thismanyboyfriends2 marked this pull request as draft January 3, 2026 02:19
@thismanyboyfriends2 thismanyboyfriends2 marked this pull request as ready for review January 4, 2026 16:33
@thismanyboyfriends2 thismanyboyfriends2 marked this pull request as draft January 5, 2026 16:36
@thismanyboyfriends2
Copy link
Copy Markdown
Author

Okay I've realised this issue is a little trickier than I first realised, after testing it seems to have introduced some other issues. Let me look into it and fix it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant