Skip to content

Close chat history service in AiService #15535

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

Merged
Siedlerchr merged 2 commits into from
Apr 12, 2026
+1 −0
Merged

Close chat history service in AiService #15535

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53bdea0
Close chat history service in AiService
InAnYan Apr 12, 2026
22c5806
Update AiService.java
calixtus Apr 12, 2026
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
1 change: 1 addition & 0 deletions jablib/src/main/java/org/jabref/logic/ai/AiService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ public void setupDatabase(BibDatabaseContext context) {
public void close() {
shutdownSignal.set(true);

chatHistoryService.close();
Copy link
Copy Markdown
Contributor

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Apr 12, 2026

Choose a reason for hiding this comment

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

Action required

1. Shutdown stops on exception 🐞 Bug ☼ Reliability

AiService.close() now calls chatHistoryService.close() before shutting down the executor/models; if
chatHistoryService.close() throws while persisting MVStore data, the remaining shutdown steps are
skipped. This can leave thread pools/models/storages unclosed and can also make try-with-resources
call sites fail during cleanup.
Agent Prompt 
### Issue description
`AiService.close()` now calls `chatHistoryService.close()` early, but the method has no exception-safety. If `chatHistoryService.close()` throws (e.g., during MVStore commit/close), the rest of `AiService.close()` is skipped, leaving executor/models/stores unclosed.

### Issue Context
`ChatHistoryService.close()` performs persistence and then commits/closes its MVStore-backed storage. The MVStore wrapper (`MVStoreBase`) does not catch exceptions.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/ai/AiService.java[136-149]
- jablib/src/main/java/org/jabref/logic/ai/chatting/ChatHistoryService.java[194-204]
- jablib/src/main/java/org/jabref/logic/ai/util/MVStoreBase.java[55-65]

### Suggested fix
Refactor `AiService.close()` to always execute the remaining shutdown steps even if chat history persistence fails. For example:
- Wrap `chatHistoryService.close()` in a `try { ... } catch (Exception e) { ... }` (log) and continue, **or**
- Use a `try/finally` where `chatHistoryService.close()` is in the `try` and the remaining shutdown steps are in `finally`, optionally collecting and rethrowing a final exception with suppressed exceptions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

cachedThreadPool.shutdownNow();
jabRefChatLanguageModel.close();
jabRefEmbeddingModel.close();
Expand Down
Loading