[Resource Leak] BufferedReader created repeatedly in loop without closing in FFmpegExecuteAsyncTask

[CyberSecurity-NCC]

Summary

The method checkAndUpdateProcess() instantiates a new BufferedReader (wrapping process.getErrorStream()) on every iteration of its polling loop. These reader instances are never closed.

This implementation leads to a continuous creation of unclosed I/O streams as long as the FFmpeg process is running, causing resource leaks and high memory churn.

• Affected File: FFmpegAndroid/src/main/java/com/github/hiteshsondhi88/libffmpeg/FFmpegExecuteAsyncTask.java
• Class: FFmpegExecuteAsyncTask
• Method: private void checkAndUpdateProcess()
• Lines: 77-102

Impact

• Resource Leak: The BufferedReader and underlying InputStreamReader are created repeatedly but never closed. This relies entirely on the Garbage Collector (GC) to clean up, which is unpredictable and can lead to resource exhaustion (e.g., File Descriptors) in long-running processes.
• Memory Overhead: Creating a new BufferedReader (and its internal buffer arrays) on every loop iteration generates unnecessary object allocation and memory churn, increasing GC pressure.
• Potential Data Loss & Blocking: Recreating the BufferedReader inside the loop discards its internal buffer (default 8KB), which may cause log data to be lost between iterations. Additionally, because readLine() blocks, placing the timeout check inside the loop means the timeout logic might be bypassed if the stream stalls.

Code Analysis

Current Implementation:

while (!Util.isProcessCompleted(process)) {
    // ...
    try {
        // ISSUE: A new BufferedReader is allocated on EVERY loop iteration
        // and it is NEVER closed.
        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getErrorStream()));
        String line;
        while ((line = reader.readLine()) != null) {
             // ... processing ...
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

Suggested Fix

Instantiate the BufferedReader once outside the loop and use try-with-resources (or a try-finally block) to ensure it is closed properly when the task finishes. This avoids repeated object creation and ensures deterministic resource cleanup.

Patch

private void checkAndUpdateProcess() throws TimeoutException, InterruptedException {
    // Fix: Create reader once, outside the loop, using try-with-resources
    try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getErrorStream()))) {
        String line;
        while (!Util.isProcessCompleted(process)) {
            // Check timeout
            if (timeout != Long.MAX_VALUE && System.currentTimeMillis() > startTime + timeout) {
                throw new TimeoutException("FFmpeg timed out");
            }

            // Read available output
            while ((line = reader.readLine()) != null) {
                if (isCancelled()) {
                    return;
                }
                output += line + "\n";
                publishProgress(line);
            }
        }
    } catch (IOException e) {
        android.util.Log.e("FFmpeg", "Error reading output", e);
    }
}

Context & Acknowledgement:

This issue was identified during our academic research on Java resource management. We have manually verified this finding.

Thank you for maintaining ffmpeg-android-java! We hope this report helps improve the project's code quality.