[versun]

Summary

These are four older bugs in the scheduled feed updater and TaskManager. I found them while reviewing #166, but #166 didn't introduce any of them. Each fix is a separate commit with a regression test. The branch is rebased onto main (cdb997d). Every new regression test fails on cdb997d (and on 5a74228) and passes on this branch.

Commit	Severity	Bug
Remove TaskManager executor restart to fix submit deadlock	high	Cron hangs forever once a frequency has more than 200 feeds. Admin "Force update" can wedge the web process's TaskManager.
Key scheduled feed update tasks by slug instead of name	medium	When two feeds have the same name, scheduled runs only fetch the first one.
Handle feedparser results without a status in fetch_feed	low	Network failures are logged as "object has no attribute 'status'" and the httpx fallback is skipped.
Keep the lock holder PID when another updater run competes	low	Every competing cron run erases the PID in the lock file.

1. TaskManager deadlock at the 201st submission (high)

Root cause. submit_task called _restart_executor() while holding self.lock. _restart_executor() calls executor.shutdown(wait=True), which joins the worker threads. Each worker needs self.lock in _wrap_task to record task status, so neither side can proceed. The default restart_threshold=200 meant the 201st submission triggered it:

• Cron (feed_updater). Feed.update_frequency defaults to 30, so most feeds land in one bucket. Once that bucket has more than 200 feeds, the command blocks in submit_task forever while holding the frequency flock. That frequency then gets no updates until the container restarts.
• Web process (admin "Force update"). update_multiple_feeds submits from inside a TaskManager worker. When those nested submissions cross the threshold, the worker tries to shut down its own pool. Depending on join order, it either deadlocks the whole TaskManager or fails with RuntimeError: cannot join current thread, and the remaining feeds are never submitted.

Evidence (real TaskManager with production defaults and the real update_multiple_feeds; update_single_feed replaced by a 50 ms sleep):

probe_restart_deadlock.py 200 -> OK: finished in 1.09s; completed tasks=200
probe_restart_deadlock.py 201 -> HANG: update_multiple_feeds still running after 10.0s; completed tasks=10
  Thread-1 (run) <- threading.py:join <- thread.py:shutdown <- task_manager.py:63:_restart_executor <- task_manager.py:99:submit_task
  task_manager__0..9 <- task_manager.py:170:_wrap_task          (all 10 workers waiting on self.lock)
probe_force_update_restart.py (threshold=5) -> update_multiple_feeds logged: 'cannot join current thread'

Fix. Remove the restart mechanism (restart_threshold, tasks_executed_since_restart, _restart_executor). Nothing ever showed that the restart prevented a memory leak. ThreadPoolExecutor already reuses its worker threads, and _cleanup_tasks already caps tasks/futures at max_task_history. The tests that covered the restart behavior are removed.

Tests. TaskManagerTestCase.test_submit_more_tasks_than_old_restart_threshold_while_running submits 250 tasks while an earlier task is still running. test_nested_submissions_from_worker_do_not_deadlock runs the same burst from inside a worker, which is the "Force update" path. Both run in a subprocess with a 15 s timeout, so a regression fails the test instead of hanging the test runner. A deadlocked executor's non-daemon workers would otherwise block interpreter exit. On 5a74228 both fail with TaskManager deadlocked during a direct/nested burst of 250 tasks. On this branch each takes about 0.5 s.

After the fix, the same probes give 200 feeds in 1.16 s, 201 feeds in 1.21 s and 1000 feeds in 5.80 s. The Force update probe with 250 nested feeds gives outer done: True, later submit: OK, with no exception logged.

2. Same-name feeds are deduplicated in scheduled updates (medium)

Root cause. update_multiple_feeds submitted update_feed_{feed.name}. Feed.name is neither unique nor required, and it is filled in from the feed title. The unique key is (feed_url, target_language), so subscribing to one source in several languages produces feeds with the same name. TaskManager.submit_task returns the existing future when a task with that name is pending or running. As a result, every same-named feed after the first was skipped on every scheduled run, though it was still re-cached.

Evidence. probe_dup_names.py submitted ['hn-zh', 'hn-ja'] but only updated ['hn-zh']. cache_rss was called for both.

Fix. Key the task by feed.slug, which is unique and set automatically on save. This matches the key the admin single-feed update already uses (core/admin/feed_admin.py: update_feed_{feed.slug}). Two exact-name assertions in existing tests were updated.

Test. UpdateMultipleFeedsTests.test_feeds_with_same_name_are_all_updated creates two real Feed rows with the same name and URL in different target languages, then runs them through a real TaskManager. On 5a74228 only one feed is updated. After the fix the probe updates both (['hn-ja', 'hn-zh']).

3. fetch_feed loses the real error on network failures (low)

Root cause. When feedparser hits a URLError (DNS failure, connection refused, connect or TLS-handshake timeout), it returns a bozo result without a status key (feedparser/api.py, parse). fetch_feed read feed.status unconditionally, so it raised AttributeError. The feed log then showed object has no attribute 'status', and the manual_fetch_feed (httpx) fallback never ran. Since #166 added fetch timeouts, connect and TLS timeouts end up on this path too.

Evidence (probe_https_connect.py, RSS_FETCH_TIMEOUT=0.5):

before: TLS handshake stall  -> error="object has no attribute 'status'" manual_fallback_called=False
before: connect stall        -> error="object has no attribute 'status'" manual_fallback_called=False
after:  TLS handshake stall  -> error='Timeout while requesting https://127.0.0.1:…/feed' manual_fallback_called=True
after:  connect stall        -> error='Timeout while requesting http://10.255.255.1/feed' manual_fallback_called=True
after:  connection refused   -> 'Error while requesting http://127.0.0.1:…/feed: [Errno 61] Connection refused'

Fix. Use feed.get("status") == 304. URLError results now go through the existing "bozo and no entries" branch to manual_fetch_feed, which reports the real error. The fallback is kept on purpose: httpx and urllib can differ in SSL/CA handling, so the second attempt can succeed. The trade-off is that a feed that is unreachable on both paths takes up to two timeouts to fail instead of one (about 1.0 s instead of 0.5 s in the probe). The DummyFeed test fixture now exposes its attributes through get(), like FeedParserDict. Without that, its 304 case would no longer be seen.

Test. FetchFeedTests.test_connection_failure_reports_real_error_and_falls_back fetches from a closed localhost port. It checks that the fallback ran and that the error is the connection failure. On 5a74228 it fails with Expected 'manual_fetch_feed' to have been called once. Called 0 times.

4. Competing cron runs erase the lock-holder PID (low)

Root cause. Command.handle opened the lock file with mode "w" before calling flock. Every competing run therefore truncated the file, even though it then failed to get the lock and exited. Since #166 the lock is held until all workers finish, so the PID is what you need to find a stuck updater, and that is when it was being erased.

Evidence. Here a real Command.handle competes against a holder (the lock path is redirected to a temp dir):

5a74228: holder wrote '10827' -> competing run exited 0 -> file content ''
branch:  holder wrote '10835' -> competing run exited 0 -> file content '10835'

Fix. Open the file in append mode, which does not truncate. Truncate it only after the lock is acquired, just before writing the PID.

Tests. UpdateFeedsLockRegressionTests.test_competing_run_keeps_holder_pid fails on 5a74228 with '' != '4242'. test_holder_pid_replaces_stale_lock_file_content checks that the new holder replaces longer stale content. Two existing mock_open assertions now expect "a". The lock-lifetime tests in test_feed_update_regressions.py pass unchanged.

Testing

• python manage.py test passes at every commit: 667, 668, 669 and 671 tests OK. The baseline on 5a74228 is 667 OK.
• git diff --check is clean.
• Rebased onto main (cdb997d, the Keep the updater lock until feed workers finish #166 follow-up); the rebased branch passes all 671 tests.

[cloudflare-workers-and-pages]

Deploying rss-translator-home with    Cloudflare Pages

Latest commit:	363bd9b
Status:	✅  Deploy successful!
Preview URL:	https://e495119c.rss-translator-home.pages.dev
Branch Preview URL:	https://fix-feed-updater-preexisting.rss-translator-home.pages.dev

View logs