Skip to content

feat(feeds): add per-channel timeout to FEED_REFRESH loop #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jollySleeper wants to merge 1 commit into
base: master
Choose a base branch
from
+23 −9
Open

feat(feeds): add per-channel timeout to FEED_REFRESH loop #892

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions src/main/java/me/kavin/piped/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
Comment on lines +34 to +36

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Java 21, does CompletableFuture.get(long, TimeUnit) cancel or interrupt the asynchronous computation on TimeoutException, and does FutureTask.cancel(true) interrupt the runner?

💡 Result:

In Java 21, the behavior is as follows: 1. CompletableFuture.get(long, TimeUnit) and TimeoutException: Calling CompletableFuture.get(long, TimeUnit) does not cancel or interrupt the underlying asynchronous computation when a TimeoutException is thrown [1][2]. The TimeoutException only indicates that the calling thread has stopped waiting for the result; the task continues to execute in the background [1][2]. To stop the computation, you must explicitly call cancel(true) on the CompletableFuture instance after catching the TimeoutException [1][2]. Alternatively, you can use the non-blocking orTimeout(long, TimeUnit) method to cause the CompletableFuture to complete exceptionally with a TimeoutException if it does not finish within the specified time [3][4]. 2. FutureTask.cancel(true) and the Runner: Yes, FutureTask.cancel(true) attempts to interrupt the thread currently executing the task [5][6]. If the task has already started, passing true to the cancel method signals that the thread running the task should be interrupted [5][6]. The implementation sets the task state to INTERRUPTING and calls thread.interrupt on the runner [7][8]. However, the success of this interruption depends on whether the task code is responsive to interrupts (e.g., it checks the interrupted status or calls blocking methods that throw InterruptedException) [1][2][9].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant section of Main.java with line numbers
file="src/main/java/me/kavin/piped/Main.java"
wc -l "$file"
sed -n '260,320p' "$file" | cat -n

Repository: TeamPiped/Piped-Backend

Length of output: 4114

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the imports and the feed refresh block in Main.java
file="src/main/java/me/kavin/piped/Main.java"
sed -n '1,60p' "$file" | cat -n
printf '\n---\n'
sed -n '280,315p' "$file" | cat -n

Repository: TeamPiped/Piped-Backend

Length of output: 5590

Make the feed refresh job cancellable.

  • CompletableFuture.runAsync(...).get(60, TimeUnit.SECONDS) only times out the waiter; the refresh keeps running on the common pool and can still mutate channel state after being skipped. Use an interruptible task/thread and cancel it on timeout.
  • The current InterruptedException handler only breaks the inner channel loop, so an interrupt does not stop the outer refresh loop.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/me/kavin/piped/Main.java` around lines 34 - 36, The feed
refresh job in Main.refresh needs to be truly cancellable instead of only timing
out the wait on CompletableFuture.runAsync(...).get(60, TimeUnit.SECONDS). Run
the refresh work on an interruptible task/thread, keep a handle to it, and
explicitly cancel or interrupt it when the timeout is hit so the work stops
before mutating channel state. Also update the InterruptedException handling in
the refresh loop so it exits the outer refresh flow, not just the inner channel
loop, when an interrupt is received.

import java.util.regex.Pattern;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -278,15 +280,27 @@ public void run() {

for (String channelId : channelIds) {
try {
var info = ChannelInfo.getInfo("https://youtube.com/channel/" + channelId);
var tabInfo = ChannelHelpers.videosTabInfo(info);
if (tabInfo != null)
Multithreading.runAsync(() -> ChannelHelpers.federateChannelVideos(tabInfo));
Multithreading.runAsync(() -> ChannelHelpers.federateChannelInfo(info));
if (tabInfo != null)
ChannelHelpers.updateChannelVideos(info, tabInfo);
} catch (Exception e) {
ExceptionHandler.handle(e);
CompletableFuture.runAsync(() -> {
try {
var info = ChannelInfo.getInfo("https://youtube.com/channel/" + channelId);
var tabInfo = ChannelHelpers.videosTabInfo(info);
if (tabInfo != null)
Multithreading.runAsync(() -> ChannelHelpers.federateChannelVideos(tabInfo));
Multithreading.runAsync(() -> ChannelHelpers.federateChannelInfo(info));
if (tabInfo != null)
ChannelHelpers.updateChannelVideos(info, tabInfo);
} catch (Exception e) {
throw new RuntimeException(e);
}
}).get(60, TimeUnit.SECONDS);
} catch (TimeoutException e) {
System.out.println("FeedRefresh: timeout refreshing channel " + channelId + ", skipping");
} catch (ExecutionException e) {
var cause = e.getCause();
ExceptionHandler.handle(cause instanceof Exception ex ? ex : e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
break;
Comment on lines +301 to +303

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Exit the refresh thread on interruption.

Line 303 only breaks out of the for loop. Because the interrupt flag is restored, the surrounding while (true) can immediately start another DB query/refresh pass instead of shutting down cleanly.

Proposed fix 
                             } catch (InterruptedException e) {
                                 Thread.currentThread().interrupt();
-                                break;
+                                return;
                             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
break;
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/me/kavin/piped/Main.java` around lines 301 - 303, The
interruption handling in Main’s refresh loop only breaks the inner for-loop, so
the surrounding while(true) can continue after
Thread.currentThread().interrupt(). Update the InterruptedException catch in the
refresh thread logic to exit the entire refresh thread cleanly when interrupted,
using the refresh loop around the DB query/refresh pass as the target to stop
rather than only the inner loop.

}
Thread.sleep(delay);
}
Expand Down
Loading