diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 85f3dd6..b121749 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -16,7 +16,7 @@ {"id":"smartfiles-blx","title":"FileAlreadyExistsException when importing duplicate filenames","description":"In ArchiveServiceImpl.manageFiles() (line 52), Files.copy() is called without specifying behavior for existing files. If a file with the same name already exists in the archive, this will throw FileAlreadyExistsException.\n\n**Fix:** Either:\n1. Use REPLACE_EXISTING option\n2. Generate unique filename (add suffix)\n3. Prompt user for action","status":"open","priority":2,"issue_type":"bug","created_at":"2026-01-15T22:36:23.292807+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:36:23.292807+01:00"} {"id":"smartfiles-bvs","title":"Path separator hardcoded breaks Windows compatibility","description":"ArchiveEntry.of() (line 29) uses hardcoded '/' to extract filename: absolutePath.substring(absolutePath.lastIndexOf('/') + 1). This will fail on Windows where paths use backslashes.\n\nMeanwhile, SmartFilesConfiguration.java correctly uses FileSystems.getDefault().getSeparator().\n\n**Fix:** Use File.separator or Path.getFileName() instead of hardcoded '/'.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-01-15T22:36:11.37771+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:36:11.37771+01:00"} {"id":"smartfiles-djt","title":"Missing test coverage for UI components","description":"There are no tests for:\n- ApplicationViewBuilder\n- ApplicationInteractor\n- DocumentView\n- PdfImageRenderer\n- TagFilterView\n- WrappingListView\n- DocumentListCell\n- TagListCell\n\nWhile UI testing can be complex, at minimum the ApplicationInteractor event handling logic should be unit tested.\n\n**Fix:** Add unit tests for ApplicationInteractor event handling. Consider TestFX for UI component tests.","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-15T22:37:10.739326+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:37:10.739326+01:00"} -{"id":"smartfiles-h74","title":"PDDocument resources never closed causing memory leak","description":"In PdfImageRenderer.java, PDDocument objects are loaded but never closed. The renderers HashMap in DocumentView.java caches PdfImageRenderer instances indefinitely. \n\nPDFBox documents hold native resources that should be explicitly closed to prevent memory leaks, especially when opening many different PDFs.\n\n**Files:**\n- PdfImageRenderer.java (line 22-25)\n- DocumentView.java (line 26)\n\n**Fix:** Implement proper resource lifecycle management. Consider:\n1. Closing PDDocument when switching documents\n2. Implementing a cache eviction strategy\n3. Using try-with-resources or explicit close on application exit","status":"open","priority":1,"issue_type":"bug","created_at":"2026-01-15T22:32:53.008106+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:32:53.008106+01:00"} +{"id":"smartfiles-h74","title":"PDDocument resources never closed causing memory leak","description":"In PdfImageRenderer.java, PDDocument objects are loaded but never closed. The renderers HashMap in DocumentView.java caches PdfImageRenderer instances indefinitely. \n\nPDFBox documents hold native resources that should be explicitly closed to prevent memory leaks, especially when opening many different PDFs.\n\n**Files:**\n- PdfImageRenderer.java (line 22-25)\n- DocumentView.java (line 26)\n\n**Fix:** Implement proper resource lifecycle management. Consider:\n1. Closing PDDocument when switching documents\n2. Implementing a cache eviction strategy\n3. Using try-with-resources or explicit close on application exit","status":"in_progress","priority":1,"issue_type":"bug","created_at":"2026-01-15T22:32:53.008106+01:00","created_by":"abedurftig","updated_at":"2026-01-16T21:07:10.44859+01:00"} {"id":"smartfiles-hj5","title":"Display the dateCreated and dateLastModified of the archive itself","description":"Create a footer area across the full width and display the dateCreated and dateLastModified of the Archive itself","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-01-07T21:33:33.098999+01:00","created_by":"abedurftig","updated_at":"2026-01-07T21:53:02.388736+01:00","closed_at":"2026-01-07T21:53:02.38875+01:00"} {"id":"smartfiles-i6y","title":"Add configurable inbox folder setting","description":"Introduce an inbox folder feature where smartfiles will automatically import PDF files after startup. The folder path is configurable in the settings dialog with the default being the user's Downloads folder on macOS (~/).\n\n**Scope for this issue (first step):**\n- Add inbox folder path to persisted settings (settings.json)\n- Add inbox folder configuration to the settings dialog\n- When folder is modified and confirmed, persist to settings.json\n- Default value: user home downloads folder on macOS\n\n**Future work (separate issues):**\n- Automatic import of PDFs from inbox folder on startup","status":"closed","priority":2,"issue_type":"feature","created_at":"2026-01-11T22:01:36.878412+01:00","created_by":"abedurftig","updated_at":"2026-01-11T22:04:23.85807+01:00","closed_at":"2026-01-11T22:04:23.85807+01:00","close_reason":"Closed"} {"id":"smartfiles-mdx","title":"PDF rendering uses fixed high DPI regardless of display","description":"PdfImageRenderer.renderPage() (line 40) uses dpi = 150 * 1.0 * 2.0 = 300 DPI unconditionally. This creates large images even for standard displays, consuming more memory and CPU than necessary.\n\n**Fix:** Detect actual display DPI and scale appropriately, or make DPI configurable.","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-15T22:37:04.349155+01:00","created_by":"abedurftig","updated_at":"2026-01-15T22:37:04.349155+01:00"} diff --git a/src/main/java/dev/arne/smartfiles/app/components/DocumentView.java b/src/main/java/dev/arne/smartfiles/app/components/DocumentView.java index f9ad521..5b369cd 100644 --- a/src/main/java/dev/arne/smartfiles/app/components/DocumentView.java +++ b/src/main/java/dev/arne/smartfiles/app/components/DocumentView.java @@ -9,6 +9,7 @@ import org.slf4j.Logger; import java.io.File; +import java.io.IOException; import java.nio.channels.ClosedByInterruptException; import java.nio.channels.ClosedChannelException; import java.util.ArrayList; @@ -23,9 +24,10 @@ public class DocumentView extends ScrollPane { private final Logger logger = org.slf4j.LoggerFactory.getLogger(DocumentView.class); private final VBox content = new VBox(); - private final Map renderers = new HashMap<>(); private final List> activeTasks = new ArrayList<>(); private final Map currentImageViews = new HashMap<>(); + private PdfImageRenderer currentRenderer; + private String currentFilePath; public DocumentView() { super(); @@ -43,6 +45,16 @@ public void clear() { activeTasks.clear(); currentImageViews.clear(); content.getChildren().clear(); + + if (currentRenderer != null) { + try { + currentRenderer.close(); + } catch (IOException e) { + logger.warn("Failed to close PDF renderer", e); + } + currentRenderer = null; + currentFilePath = null; + } } public void viewFile(File file) { @@ -59,24 +71,39 @@ public void viewFile(File file) { currentImageViews.clear(); this.setVvalue(0.0); - PdfImageRenderer renderer = renderers.get(file.getAbsolutePath()); - if (renderer == null) { + + String filePath = file.getAbsolutePath(); + + // Close previous renderer if switching to a different file + if (currentRenderer != null && !filePath.equals(currentFilePath)) { + try { + currentRenderer.close(); + } catch (IOException e) { + logger.warn("Failed to close previous PDF renderer", e); + } + currentRenderer = null; + currentFilePath = null; + } + + // Create new renderer if needed + if (currentRenderer == null) { try { - renderer = new PdfImageRenderer(file); + currentRenderer = new PdfImageRenderer(file); + currentFilePath = filePath; } catch (Exception e) { + logger.error("Failed to open PDF file: {}", filePath, e); return; } - renderers.put(file.getAbsolutePath(), renderer); } content.getChildren().clear(); - var number = renderer.getNumberOfPages(); + var number = currentRenderer.getNumberOfPages(); for (int i = 0; i < number; i++) { ImageView imageView = createPlaceholder(); currentImageViews.put(i, imageView); // Create and submit the rendering task - Task renderTask = createRenderTask(renderer, i); + Task renderTask = createRenderTask(currentRenderer, i); APP_EXECUTOR.submit(renderTask); activeTasks.add(renderTask); // Track the new task } diff --git a/src/main/java/dev/arne/smartfiles/app/pdf/PdfImageRenderer.java b/src/main/java/dev/arne/smartfiles/app/pdf/PdfImageRenderer.java index ac276e6..66110ef 100644 --- a/src/main/java/dev/arne/smartfiles/app/pdf/PdfImageRenderer.java +++ b/src/main/java/dev/arne/smartfiles/app/pdf/PdfImageRenderer.java @@ -8,11 +8,12 @@ import org.apache.pdfbox.rendering.PDFRenderer; import java.awt.image.BufferedImage; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.util.HashMap; -public final class PdfImageRenderer { +public final class PdfImageRenderer implements Closeable { private final PDDocument document; private final PDFRenderer renderer; @@ -46,4 +47,12 @@ public Image renderPage(int pageIndex) throws IOException { return result; } } + + @Override + public void close() throws IOException { + imageCache.clear(); + if (document != null) { + document.close(); + } + } }