jabref icon indicating copy to clipboard operation
jabref copied to clipboard

Fix for postgres instance process closing

Open UdayHE opened this issue 8 months ago • 12 comments

Fixes https://github.com/JabRef/jabref/issues/12844

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

  • [x] I own the copyright of the code submitted and I license it under the MIT license.
  • [x] 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)
  • [x] Manually tested changed features in running JabRef (always required)
  • [x] 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

UdayHE avatar Apr 13 '25 05:04 UdayHE

This is AI generated, isn't it?

koppor avatar Apr 13 '25 07:04 koppor

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

subhramit avatar Apr 13 '25 07:04 subhramit

This is AI generated, isn't it?

nope.

UdayHE avatar Apr 13 '25 07:04 UdayHE

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.

UdayHE avatar Apr 13 '25 07:04 UdayHE

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.

subhramit avatar Apr 13 '25 09:04 subhramit

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?

https://github.com/user-attachments/assets/6e50e8c3-c37a-4bdd-b4db-5cd1c09ea1d1

UdayHE avatar Apr 15 '25 08:04 UdayHE

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-<javaPid>.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.

UdayHE avatar Apr 15 '25 09:04 UdayHE

Your pull request needs to link an issue.

To ease organizational workflows, please link this pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue:

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message. The pull request must be on the default branch.

  • close
  • closes
  • closed
  • fix
  • fixes
  • fixed
  • resolve
  • resolves
  • resolved

If you use a keyword to reference a pull request comment in another pull request, the pull requests will be linked. Merging the referencing pull request also closes the referenced pull request.

The syntax for closing keywords depends on whether the issue is in the same repository as the pull request.

Examples

  • Fixes #xyz links pull-request to issue. Merging the PR will close the issue.
  • Fixes https://github.com/JabRef/jabref/issues/xyz links pull-request to issue. Merging the PR will close the issue.
  • Fixes https://github.com/Koppor/jabref/issues/xyz links pull-request to issue. Merging the PR will close the issue.
  • Fixes [#xyz](https://github.com/JabRef/jabref/issues/xyz) links pull-request to issue. Merging the PR will NOT close the issue.

jabref-machine avatar Apr 16 '25 08:04 jabref-machine

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.

koppor avatar May 11 '25 20:05 koppor

Below is the flow and classes involved. stop_stale_postgres.txt

Few doubts I have, please find below

  1. Are we going to disallow multiple jabref instances?
  2. 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.

UdayHE avatar May 18 '25 10:05 UdayHE

Below is the flow and classes involved. stop_stale_postgres.txt

Few doubts I have, please find below

  1. Are we going to disallow multiple jabref instances?

I see two options:

  1. 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
  2. Keep the concept as is.

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

  1. 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

koppor avatar May 18 '25 10:05 koppor

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

trag-bot[bot] avatar May 20 '25 03:05 trag-bot[bot]

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

koppor avatar Jul 07 '25 21:07 koppor