Fix for postgres instance process closing #12927

UdayHE · 2025-04-13T05:13:53Z

Summary

Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions.
Introduced PostgreProcessCleaner to safely shut down any orphaned PostgreSQL processes using port-based detection and Java PID tracking.
Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdowns (excluding SIGKILL).

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license.
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

macOS Sequoia 15.4.
https://www.loom.com/share/9ed0da7815714797b4f0568c33f6df03?sid=34114b6c-ca32-43ff-9896-bfae39b36c81

Windows
https://github.com/user-attachments/assets/aa7a7da4-661e-464e-a08f-f6befba66363

…ination (Fix JabRef#12844) * Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions. * Introduced PostgresProcessCleaner to safely shut down any orphaned PostgreSQL processes using PID-based detection. * Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdown.

…ination (Fix JabRef#12844) * Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions. * Introduced PostgresProcessCleaner to safely shut down any orphaned PostgreSQL processes using PID-based detection. * Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdown. * Updated CHANGELOG.md

…ination (Fix JabRef#12844) * Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions. * Introduced PostgresProcessCleaner to safely shut down any orphaned PostgreSQL processes using PID-based detection. * Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdown.

…ination (Fix JabRef#12844) * Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions. * Introduced PostgresProcessCleaner to safely shut down any orphaned PostgreSQL processes using PID-based detection. * Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdown. * Updated CHANGELOG.md

…ssue-12844

…ination (Fix JabRef#12844) * Added logic to detect and clean up stale embedded PostgreSQL instances left behind by previous JabRef sessions. * Introduced PostgresProcessCleaner to safely shut down any orphaned PostgreSQL processes using PID-based detection. * Registered a shutdown hook in Launcher to ensure embedded PostgreSQL is properly terminated during normal or abrupt shutdown. * Updated CHANGELOG.md

…ination (Fix JabRef#12844) * style changes in PostgreProcessCleaner, Launcher

koppor · 2025-04-13T06:58:02Z

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

+
+    private void destroyPreviousJavaProcess(Map<String, Object> meta) throws InterruptedException {
+        long javaPid = ((Number) meta.get("javaPid")).longValue();
+        destroyProcessByPID(javaPid, 1000);


How does one know whether this stores JabRef data? Shouldn't a connection attempt be made?

In writeMetadataToFile method, I am using ProcessHandle.current().pid(), so that the current running process id will be stored in the json file.

koppor · 2025-04-13T07:01:43Z

This is AI generated, isn't it?

subhramit · 2025-04-13T07:05:04Z

Have you tested this by observing the processes? If so, on which OS?
I tried on Windows. Doesn't work.

UdayHE · 2025-04-13T07:18:13Z

This is AI generated, isn't it?

nope.

koppor · 2025-04-13T07:48:13Z

src/main/java/org/jabref/logic/search/PostgreServer.java


 public class PostgreServer {
    private static final Logger LOGGER = LoggerFactory.getLogger(PostgreServer.class);
+    public static final Path POSTGRES_METADATA_FILE = Path.of("/tmp/jabref-postgres-info.json");


Will not work in Windows. Will not work if multiple JabRef instances run.

For the former, AppDirs can be used (can't it?) - for the latter, the whole flow needs to be thought through.

Got it, I need to check about AppDirs once & get back on this.

UdayHE · 2025-04-13T07:51:04Z

Have you tested this by observing the processes? If so, on which OS? I tried on Windows. Doesn't work.

yes, I have tested on macOS Sequoia 15.4.
https://www.loom.com/share/9ed0da7815714797b4f0568c33f6df03?sid=34114b6c-ca32-43ff-9896-bfae39b36c81

I will check on windows and update here.

subhramit · 2025-04-13T09:23:15Z

yes, I have tested on macOS Sequoia 15.4.
https://www.loom.com/share/9ed0da7815714797b4f0568c33f6df03?sid=34114b6c-ca32-43ff-9896-bfae39b36c81

Interesting - this seems to be going in a positive direction, if we can fix it for Windows.
Let me know if you want me to test and give feedback after a certain round of changes.

…ination (Fix JabRef#12844) * style changes in PostgreServer imports

…ination (Fix JabRef#12844) * Updated CHANGELOG.md

…ination (Fix JabRef#12844) * Modified PostgreProcessCleaner logic to fix windows os issue.

src/main/java/org/jabref/Launcher.java

UdayHE · 2025-04-15T08:55:10Z

Have you tested this by observing the processes? If so, on which OS? I tried on Windows. Doesn't work.

checked on windows as well, can you please check from your end as well?

screen-capture-windows.mp4

…ination (Fix JabRef#12844) * Used Path.of() instead of Paths.get() in PostgreProcessCleaner.

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

UdayHE · 2025-04-15T09:15:44Z

Embedded Postgres Cleanup Flow
1.Start the Application
When a new JabRef instance starts, it initiates the embedded Postgres server.

2.Write Metadata File
During startup, the application writes a JSON file to the temporary directory (java.io.tmpdir).
File naming convention: jabref-postgres-info-.json (e.g., jabref-postgres-info-1234.json).
This file contains:
javaPid: The PID of the current Java process.
postgresPort: The port used by the embedded Postgres instance.
etc..

3.Check for Metadata Files on Startup
On startup, the application scans the temp directory for any matching metadata files (jabref-postgres-info-*.json).
This supports scenarios where multiple instances may have left behind multiple metadata files.
Read Stored Process Details.
For each metadata file found:
It reads the javaPid and postgresPort from the JSON.

4.Verify Java Process Liveness
For each javaPid, the application checks whether the corresponding Java process is still alive.

5.Handle Stale Java Processes
If any Java process is no longer running:
It indicates that a previous JabRef instance was terminated (e.g., via kill -9 or SIGKILL).

6.Lookup Postgres Processes
For each stale entry:
The application performs a port-based lookup to check if any process is still listening on the postgresPort.

7.Kill Stale Postgres
If a Postgres process is found on that port:
It is considered stale and is safely terminated.

8.Delete Metadata Files
After handling each stale process:
The corresponding metadata file (jabref-postgres-info-.json) is deleted.
Metadata files for live Java processes are retained, to avoid interfering with other active instances.

9.Proceed with Fresh Initialization
The current instance proceeds to launch a new embedded Postgres server.
A new metadata file is created specific to this instance.

10. Multiple Instance Safety
Each JabRef instance maintains its own metadata file, isolated by Java PID.
Stale cleanup only affects processes no longer alive, ensuring that active instances are never disrupted.

…ination (Fix JabRef#12844) * Fix for trag-bot comments.

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

InAnYan · 2025-04-16T14:17:16Z

src/main/java/org/jabref/logic/search/PostgresMetadataWriter.java

+        Map<String, Object> meta = new HashMap<>();
+        meta.put("javaPid", ProcessHandle.current().pid());
+        meta.put("postgresPort", port);
+        meta.put("startedBy", "jabref");


In some sense, trag-bot is right.

Is it possible to, maybe, make a serializable class?

koppor · 2025-05-11T20:22:59Z

I did not see any progress since weeks. I will close this PR. @UdayHE If you intend to continue here; feel free to ping us.

trag-bot · 2025-05-18T10:12:00Z

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

+    }
+
+    private void destroyProcessByPID(long pid) throws InterruptedException {
+        Optional<ProcessHandle> aliveProcess = ProcessHandle.of(pid).filter(ProcessHandle::isAlive);


The method destroyProcessByPID should use ifPresent on Optional instead of checking isPresent and then calling get.

trag-bot · 2025-05-18T10:12:07Z

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

+        if (aliveProcess.isPresent()) {
+            aliveProcess.get().destroy();


The code should use ifPresent to handle the Optional instead of checking isPresent and then calling get.

UdayHE · 2025-05-18T10:13:25Z

Below is the flow and classes involved.
stop_stale_postgres.txt

Few doubts I have, please find below

Are we going to disallow multiple jabref instances?
As this approach finds the stale postgres process using port-lookup, it uses commands like netstat, lsof to find the process & its PID, is there any other way to kill stale postgres without depending on these commands? like any library which helps to interact with OS related stuffs.
I found oshi (https://github.com/oshi/oshi), but I need to explore the possibility.

trag-bot · 2025-05-18T10:13:29Z

src/main/java/org/jabref/logic/search/PostgresMetadataWriter.java

+        meta.put("javaPid", ProcessHandle.current().pid());
+        meta.put("postgresPort", port);
+        meta.put("startedBy", "jabref");
+        meta.put("startedAt", DateTimeFormatter.ISO_INSTANT.format(Instant.now()));


The string literal key "startedAt" should be declared as a private static final constant to improve maintainability and readability, following best practices for handling literals.

trag-bot · 2025-05-18T10:13:30Z

src/main/java/org/jabref/logic/search/PostgresMetadataWriter.java

+
+    private static Map<String, Object> createMetadata(int port) {
+        Map<String, Object> meta = new HashMap<>();
+        meta.put("javaPid", ProcessHandle.current().pid());


The string literal key "javaPid" should be declared as a private static final constant to improve maintainability and readability, following best practices for handling literals.

trag-bot · 2025-05-18T10:13:33Z

src/main/java/org/jabref/logic/search/PostgresMetadataWriter.java

+    private static Map<String, Object> createMetadata(int port) {
+        Map<String, Object> meta = new HashMap<>();
+        meta.put("javaPid", ProcessHandle.current().pid());
+        meta.put("postgresPort", port);


The string literal key "postgresPort" should be declared as a private static final constant to improve maintainability and readability, following best practices for handling literals.

koppor · 2025-05-18T10:21:50Z

Below is the flow and classes involved.
stop_stale_postgres.txt

Few doubts I have, please find below

Are we going to disallow multiple jabref instances?

I see two options:

Switch to another method to detect multiple instances. Furthermore: Command passing needs to work with the new method! - One method is to use Postgres' notification system
Keep the concept as is.

The postgres notification system is not too hard; maybe a better way to habdle things.

As this approach finds the stale postgres process using port-lookup, it uses commands like netstat, lsof to find the process & its PID, is there any other way to kill stale postgres without depending on these commands? like any library which helps to interact with OS related stuffs.
I found oshi (https://github.com/oshi/oshi), but I need to explore the possibility.

oshi sounds promising

* Modified getPidUsingPort() to use OSHI. * Fix for review comments.

trag-bot · 2025-05-19T05:30:09Z

jabgui/src/main/java/org/jabref/Launcher.java

        CSLStyleLoader.loadInternalStyles();

+        // Register shutdown hook
+        Runtime.getRuntime().addShutdownHook(new Thread(postgreServer::shutdown));


Using 'new Thread()' is discouraged. Instead, 'org.jabref.logic.util.BackgroundTask' and its 'executeWith' method should be used for better resource management and consistency with the rest of the codebase.

trag-bot · 2025-05-19T05:30:53Z

jablib/src/main/java/module-info.java

    requires mslinks;
    requires org.antlr.antlr4.runtime;
    requires org.libreoffice.uno;
+    requires com.github.oshi;


The pull request title should contain a short title of the issue fixed or what the PR addresses, not just 'Fix issue xyz'. This is important for clarity and tracking purposes.

trag-bot · 2025-05-19T05:31:35Z

jablib/src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java

+    }
+
+    public void checkAndCleanupOldInstances() {
+        try (Stream<Path> files = Files.list(TEMP_DIR)) {


The try block covers too many statements, which can make it harder to identify where an exception occurs. It should cover as few statements as possible.

trag-bot · 2025-05-19T05:32:14Z

jablib/src/main/java/org/jabref/logic/search/PostgresMetadataWriter.java

+        try {
+            Path path = getMetadataFilePath();
+            OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValue(path.toFile(), createMetadata(port));
+            LOGGER.info("Postgres metadata file path: {}", path);
+        } catch (IOException e) {
+            LOGGER.warn("Failed to write Postgres metadata file.", e);
+        }


The try block covers multiple statements. It is recommended to minimize the scope of try blocks to only the statements that might throw exceptions.

trag-bot · 2025-05-19T05:32:45Z

jablib/src/main/java/org/jabref/logic/util/Directories.java

                                             OS.APP_DIR_APP_AUTHOR));
    }
+
+    public static Path getTmpDirectory() {


The method getTmpDirectory() is newly added and should not return null. It should use java.util.Optional to handle potential null values, ensuring better null safety and adherence to modern Java practices.

trag-bot · 2025-05-20T03:29:18Z

@trag-bot didn't find any issues in the code! ✅✨

koppor · 2025-07-07T21:23:51Z

I think, the issue needs to be adressed in another way.

UdayHE added 7 commits April 13, 2025 10:22

Merge branch 'issue-12844' of https://github.com/UdayHE/jabref into i…

e245d2b

…ssue-12844

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

d7b7586

…ination (Fix JabRef#12844) * style changes in PostgreProcessCleaner, Launcher

koppor reviewed Apr 13, 2025

View reviewed changes

Siedlerchr changed the title ~~Fix for Issue 12844~~ Fix for postgres instance process closing Apr 13, 2025

UdayHE added 3 commits April 13, 2025 18:17

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

a991627

…ination (Fix JabRef#12844) * style changes in PostgreServer imports

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

318f01d

…ination (Fix JabRef#12844) * Updated CHANGELOG.md

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

e85245f

…ination (Fix JabRef#12844) * Modified PostgreProcessCleaner logic to fix windows os issue.

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/Launcher.java Outdated Show resolved Hide resolved

Merge branch 'main' into issue-12844

69867cf

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

b01d4a7

…ination (Fix JabRef#12844) * Used Path.of() instead of Paths.get() in PostgreProcessCleaner.

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java Outdated Show resolved Hide resolved

UdayHE requested a review from koppor April 15, 2025 09:19

UdayHE added 3 commits April 15, 2025 15:02

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

4abd5f1

…ination (Fix JabRef#12844) * Fix for trag-bot comments.

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

ee83db3

…ination (Fix JabRef#12844) * Fix for trag-bot comments.

fix: ensure embedded PostgreSQL is cleaned up on crash or forced term…

482199b

…ination (Fix JabRef#12844) * Fix for trag-bot comments.

trag-bot bot reviewed Apr 15, 2025

View reviewed changes

src/main/java/org/jabref/logic/search/PostgreProcessCleaner.java Show resolved Hide resolved

InAnYan reviewed Apr 16, 2025

View reviewed changes

koppor marked this pull request as draft May 11, 2025 20:21

koppor closed this May 11, 2025

koppor reopened this May 17, 2025

Merge remote-tracking branch 'origin' into issue-12844

a9a192b

UdayHE closed this May 18, 2025

UdayHE reopened this May 18, 2025

trag-bot bot reviewed May 18, 2025

View reviewed changes

UdayHE added 2 commits May 19, 2025 09:48

Merge remote-tracking branch 'origin' into issue-12844

c4d90fc

Fix for issue-12844:

fa8eab9

* Modified getPidUsingPort() to use OSHI. * Fix for review comments.

trag-bot bot reviewed May 19, 2025

View reviewed changes

UdayHE added 5 commits May 19, 2025 16:18

Merge branch 'main' into issue-12844

5d252e5

Modified CHANGELOG.md.

9b5150d

Merge branch 'main' into issue-12844

9cec567

Merge branch 'main' into issue-12844

94bc898

Merge branch 'main' into issue-12844

5bc6477

koppor closed this Jul 7, 2025

		if (aliveProcess.isPresent()) {
		aliveProcess.get().destroy();

Uh oh!

Fix for postgres instance process closing #12927

Fix for postgres instance process closing #12927

Uh oh!

Conversation

UdayHE commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Mandatory checks

Uh oh!

koppor Apr 13, 2025

Choose a reason for hiding this comment

Uh oh!

UdayHE Apr 13, 2025

Choose a reason for hiding this comment

Uh oh!

koppor commented Apr 13, 2025

Uh oh!

subhramit commented Apr 13, 2025

Uh oh!

UdayHE commented Apr 13, 2025

Uh oh!

koppor Apr 13, 2025

Choose a reason for hiding this comment

Uh oh!

UdayHE Apr 13, 2025

Choose a reason for hiding this comment

Uh oh!

UdayHE commented Apr 13, 2025

Uh oh!

subhramit commented Apr 13, 2025

Uh oh!

Uh oh!

UdayHE commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

UdayHE commented Apr 15, 2025

Uh oh!

Uh oh!

InAnYan Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

koppor commented May 11, 2025

Uh oh!

trag-bot bot May 18, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 18, 2025

Choose a reason for hiding this comment

Uh oh!

UdayHE commented May 18, 2025

Uh oh!

trag-bot bot May 18, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 18, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 18, 2025

Choose a reason for hiding this comment

Uh oh!

koppor commented May 18, 2025

Uh oh!

trag-bot bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot May 19, 2025

Choose a reason for hiding this comment

Uh oh!

UdayHE commented Apr 13, 2025 •

edited

Loading

UdayHE commented Apr 15, 2025 •

edited

Loading