Skip to content

Fixed the proxy checker deadlocking once the queue empties - #6625

Open
dekrom wants to merge 1 commit into
MeteorDevelopment:masterfrom
dekrom:fix/proxy-checker-deadlock
Open

Fixed the proxy checker deadlocking once the queue empties#6625
dekrom wants to merge 1 commit into
MeteorDevelopment:masterfrom
dekrom:fix/proxy-checker-deadlock

Conversation

@dekrom

@dekrom dekrom commented Aug 30, 2026

Copy link
Copy Markdown

Type of change

  • Bug fix
  • New feature

Description

Proxies#check is a check then act race:

while (!queue.isEmpty()) {
    Proxy proxy = queue.take();

Every worker runs that loop against the same ArrayBlockingQueue. As the queue drains, several
of them can pass isEmpty() and then all call take(). One gets the last proxy, the rest block
forever, because nothing is ever added again once the work is done.

awaitTermination(Long.MAX_VALUE, ...) then never returns, refreshing = false is never
reached, and the try-with-resources close() blocks on top of that. So one refresh with the
default 8 threads parks up to 7 pool threads plus the MeteorExecutor thread for the rest of the
session, and because refreshing is stuck true both checkProxies and clean silently return
early from then on. The proxy screen just stops responding to refresh until you restart the
game.

poll() fixes the race without any extra bookkeeping: a worker only exits when it sees an empty
queue, and a worker that requeues a proxy for a retry goes straight back to poll(), so nothing
is left behind when the last one exits. offer() replaces put() for the same reason, and the
queue is sized to hold every proxy so it cannot reject.

Two smaller things in the same path:

  • refreshing = false moved into a finally, so a throw anywhere in the block cannot wedge the
    flag again.
  • threads had .min(0), and Executors.newFixedThreadPool(0) throws
    IllegalArgumentException. Setting the slider to 0 wedged refreshing immediately, in the
    same way. Minimum is now 1.

Related issues

None that I found.

How Has This Been Tested?

Reasoned out of the source rather than caught live, so I have not sat and watched a refresh hang
yet. The race is plain in check() and the stuck refreshing flag follows from it. Builds clean
against current master.

Checklist:

  • My code follows the style guidelines of this project.
  • I have added comments to my code in more complex areas.
  • I have tested the code in both development and production environments.

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.

2 participants