Mixing `threading.Thread + asyncio.run` workers with the long-lived
asyncio loop in gui/app.py is a real loop-affinity bug: the first worker
thread to call `asyncio.run` constructs a throwaway loop, which the
shared httpx clients then attach to, and the next call from the
persistent loop fails with "Event loop is closed" / "attached to a
different loop". This commit eliminates the pattern across the GUI and
adds the locking + cleanup that should have been there from the start.
Persistent loop accessor (core/concurrency.py — new)
- set_app_loop / get_app_loop / run_on_app_loop. BooruApp registers the
one persistent loop at startup; everything that wants to schedule
async work calls run_on_app_loop instead of spawning a thread that
builds its own loop. Three functions, ~30 lines, single source of
truth for "the loop".
Lazy-init lock + cleanup on shared httpx clients (core/api/base.py,
core/api/e621.py, core/cache.py)
- Each shared singleton (BooruClient._shared_client, E621Client._e621_client,
cache._shared_client) now uses fast-path / locked-slow-path lazy init.
Concurrent first-callers from the same loop can no longer both build
a client and leak one (verified: 10 racing callers => 1 httpx instance).
- Each module exposes an aclose helper that BooruApp.closeEvent runs via
run_coroutine_threadsafe(...).result(timeout=5) BEFORE stopping the
loop. The connection pool, keepalive sockets, and TLS state finally
release cleanly instead of being abandoned at process exit.
- E621Client tracks UA-change leftovers in _e621_to_close so the old
client doesn't leak when api_user changes — drained in aclose_shared.
GUI workers routed through the persistent loop (gui/sites.py,
gui/bookmarks.py)
- SiteDialog._on_detect / _on_test: replaced
`threading.Thread(target=lambda: asyncio.run(...))` with
run_on_app_loop. Results marshaled back through Qt Signals connected
with QueuedConnection. Added _closed flag + _inflight futures list:
closeEvent cancels pending coroutines and shorts out the result emit
if the user closes the dialog mid-detect (no use-after-free on
destroyed QObject).
- BookmarksView._load_thumb_async: same swap. The existing thumb_ready
signal already used QueuedConnection so the marshaling side was
already correct.
DB write serialization (core/db.py)
- Database._write_lock = threading.RLock() — RLock not Lock so a
writing method can call another writing method on the same thread
without self-deadlocking.
- New _write() context manager composes the lock + sqlite3's connection
context manager (the latter handles BEGIN / COMMIT / ROLLBACK
atomically). Every write method converted: add_site, update_site,
delete_site, add_bookmark, add_bookmarks_batch, remove_bookmark,
update_bookmark_cache_path, add_folder, remove_folder, rename_folder,
move_bookmark_to_folder, add/remove_blacklisted_tag,
add/remove_blacklisted_post, save_library_meta, remove_library_meta,
set_setting, add_search_history, clear_search_history,
remove_search_history, add_saved_search, remove_saved_search.
- _migrate keeps using the lock + raw _conn context manager because
it runs from inside the conn property's lazy init (where _write()
would re-enter conn).
- Reads stay lock-free and rely on WAL for reader concurrency. Verified
under contention: 5 threads × 50 add_bookmark calls => 250 rows,
zero corruption, zero "database is locked" errors.
Smoke-tested with seven scenarios: get_app_loop raises before set,
run_on_app_loop round-trips, lazy init creates exactly one client,
10 concurrent first-callers => 1 httpx, aclose_shared cleans up,
RLock allows nested re-acquire, multi-threaded write contention.
Sweep of defensive hardening across the core layers plus a related popout
overlay regression that surfaced during verification.
Database integrity (core/db.py)
- Wrap delete_site, add_search_history, remove_folder, rename_folder,
and _migrate in `with self.conn:` so partial commits can't leave
orphan rows on a crash mid-method.
- add_bookmark re-SELECTs the existing id when INSERT OR IGNORE
collides on (site_id, post_id). Was returning Bookmark(id=0)
silently, which then no-op'd update_bookmark_cache_path the next
time the post was bookmarked.
- get_bookmarks LIKE clauses now ESCAPE '%', '_', '\\' so user search
literals stop acting as SQL wildcards (cat_ear no longer matches
catear).
Path traversal (core/db.py + core/config.py)
- Validate folder names at write time via _validate_folder_name —
rejects '..', os.sep, leading '.' / '~'. Permits Unicode/spaces/
parens so existing folders keep working.
- saved_folder_dir() resolves the candidate path and refuses anything
that doesn't relative_to the saved-images base. Defense in depth
against folder strings that bypass the write-time validator.
- gui/bookmarks.py and gui/app.py wrap add_folder calls in try/except
ValueError and surface a QMessageBox.warning instead of crashing.
Download safety (core/cache.py)
- New _do_download(): payloads >=50MB stream to a tempfile in the
destination dir and atomically os.replace into place; smaller
payloads keep the existing buffer-then-write fast path. Both
enforce a 500MB hard cap against the advertised Content-Length AND
the running total inside the chunk loop (servers can lie).
- Per-URL asyncio.Lock coalesces concurrent downloads of the same
URL so two callers don't race write_bytes on the same path.
- Image.MAX_IMAGE_PIXELS = 256M with DecompressionBombError handling
in both converters.
- _convert_ugoira_to_gif checks frame count + cumulative uncompressed
size against UGOIRA_MAX_FRAMES / UGOIRA_MAX_UNCOMPRESSED_BYTES from
ZipInfo headers BEFORE decompressing — defends against zip bombs.
- _convert_animated_to_gif writes a .convfailed sentinel sibling on
failure to break the re-decode-on-every-paint loop for malformed
animated PNGs/WebPs.
- _is_valid_media returns True (don't delete) on OSError so a
transient EBUSY/permissions hiccup no longer triggers a delete +
re-download loop on every access.
- _referer_for() uses proper hostname suffix matching, not substring
`in` (imgblahgelbooru.attacker.com no longer maps to gelbooru.com).
- PIL handles wrapped in `with` blocks for deterministic cleanup.
API client retry + visibility (core/api/*)
- base.py: _request retries on httpx.NetworkError + ConnectError in
addition to TimeoutException. test_connection no longer echoes the
HTTP response body in the error string (it was an SSRF body-leak
gadget when used via detect_site_type's redirect-following client).
- detect.py + danbooru.py + e621.py + gelbooru.py + moebooru.py:
every previously-swallowed exception in search/autocomplete/probe
paths now logs at WARNING with type, message, and (where relevant)
the response body prefix. Debugging "the site isn't working" used
to be a total blackout.
main_gui.py
- file_dialog_platform DB probe failure prints to stderr instead of
vanishing.
Popout overlay (gui/preview.py + gui/app.py)
- preview.py:79,141 — setAttribute(WA_StyledBackground, True) on
_slideshow_toolbar and _slideshow_controls. Plain QWidget parents
silently ignore QSS `background:` declarations without this
attribute, which is why the popout overlay strip was rendering
fully transparent (buttons styled, bar behind them showing the
letterbox color).
- app.py: bake _BASE_POPOUT_OVERLAY_QSS as a fallback prepended
before the user's custom.qss in the loader. Custom themes that
don't define overlay rules now still get a translucent black
bar with white text + hairline borders. Bundled themes win on
tie because their identical-specificity rules come last in the
prepended string.
Old staggered drain (50ms per post) was added for visual polish but
made infinite scroll painfully slow — a 40-post page took 2 seconds
just to add to the grid. Thumbnails already load async via _fetch_thumbnail,
so the stagger was just delaying grid population for no real benefit.
Now all posts are added instantly in one pass with thumbnails filling in
as they arrive. Scroll trigger widened from 1 row to 3 rows from bottom
so the next page starts loading before you reach the end.
- Off: no prefetching
- Adjacent: simultaneous left/right/up/down (4 posts)
- Full page: ring expansion in all 8 directions
Dropdown in Settings > General replaces the old checkbox.
Uses 'animated' as a search tag (server handles it). Removed
client-side extension filter and backfill cap increases. Fast
and doesn't hit rate limits.
Short API pages (< limit) no longer stop the scan when animated
filter is on — keeps looking through more pages. Only stops on
truly empty API response or 50 page cap.
Removed server-side tag (wastes a search slot). Client-side
filter with 20 page backfill when animated is checked (vs 5
normally) to find enough animated posts.
Client-side filter by file extension. Works with backfill and
infinite scroll. Unchecked shows everything, checked shows only
gif/mp4/webm/mkv/mov/zip(ugoira).
Hides QSpinBox arrows (break under QSS) and adds two separate
QPushButtons with triangle characters. Theme-friendly since
they're styled as normal buttons.
Cacheless mode closes the app while clients may still reference
the old event loop. Resetting to None on startup forces fresh
client creation on the new event loop.
- Bookmarks DB now stores tag_categories as JSON
- Migration adds column to existing favorites table
- Bookmark info panel uses stored categories directly
- Falls back to library_meta if bookmark has no categories
- Tag click in info panel: clears preview, switches to Browse, searches
- Migrate library_meta to add tag_categories column
- Info panel always updates (not gated on isVisible)
- Library info shows status bar with score/rating
- Save indicator: changed elif to if so Saved always triggers
- Bookmark info panel always populated
- tag_categories stored as JSON in library_meta table
- Library and Bookmarks info panels show categorized tags
- Bookmarks falls back to library_meta for categories
- Added Unsave from Library to bookmarks right-click menu
- library_meta table stores tags, score, rating, source per post
- Metadata saved automatically when saving from Browse
- Search box in Library tab filters by tags via DB lookup
- Works with all file types
