configure -XX:ErrorFile to write JVM crash logs to app log directory #3649

[dhruvbajpai29]

Closes #3649.

Changes

• Added -XX:ErrorFile to the jpackage --java-options in the build scripts for Linux (dist/linux/build.sh), macOS (dist/mac/build.sh), and Windows (dist/windows/build.ps1).
• Configured crash log paths using jpackage runtime placeholders (@{userhome} for Linux/macOS, @{localappdata} for Windows) to align with the app’s log directory (-Dcryptomator.logDir), ensuring hs_err_pid*.log files are stored in user-specific locations: ~/.local/share/Cryptomator/logs (Linux), ~/Library/Logs/Cryptomator (macOS), and %LOCALAPPDATA%\Cryptomator (Windows).

Motivation

Addresses issue #3649 by redirecting JVM crash logs to the same directory as Cryptomator’s regular logs, simplifying log collection for support. The -XX:ErrorFile flag is set during the jpackage build phase to achieve this.

Implementation Details

• Runtime Placeholders: Used @{userhome} and @{localappdata} to dynamically resolve to the user’s log directory at runtime, overcoming the limitation noted by @infeo (user account names are unknown at build time, and jpackaged apps cannot read environment variables like $HOME). These placeholders are already used in the scripts for -Dcryptomator.logDir, ensuring consistency.
• Fallback Behavior: Per Oracle JVM documentation, if the target directory is missing or unwritable (e.g., first run), the JVM stores the crash log in the OS temp directory — still more predictable than the default (current working directory).
• Avoided Alternatives: Global writable directories (e.g., /var/log, C:\ProgramData) were not used, as they often require elevated permissions and don’t align with user-specific log locations. A “never-existing” path (e.g., /nonexistent) was also avoided, as placeholders achieve the goal directly.

Addressing issues

• @infeo highlighted the challenge of user-specific paths and jpackage’s inability to use environment variables. Runtime placeholders resolve this by expanding at launch to the correct user paths, as supported by Oracle’s jpackage documentation.
• @overheadhunter noted the lack of a $APPDIR-equivalent for user home. The placeholders (@{userhome}, @{localappdata}) serve this purpose, matching the existing script structure.

Testing

• Built the app with mvn clean install.
• Verified that -XX:ErrorFile appears in the launcher config file ( with unexpanded placeholders ( @{userhome}/...).

-wrote a Java class that used Unsafe to trigger a JVM crash, ran it with the same -XX:ErrorFile option i added to the build script, and confirmed that the crash log was created in the expected directory.

• Did not simulate JVM crashes on Cryptomator, as this requires complex setup. Relied instead on Oracle JVM documentation for -XX:ErrorFile behavior and consistency with existing placeholder usage in the scripts. Open to suggestions for safe crash testing methods.

@infeo @overheadhunter Please review and provide any feedback or suggestions.

[CLAassistant]

All committers have signed the CLA.

[overheadhunter]

The placeholders (@{userhome}, @{localappdata}) serve this purpose, matching the existing script structure.

These placeholders get resolved by application code, not JVM code. Therefore this mechanism won't work and we need to identify feasible paths as noted in the linked issue.

Alternatively, a PR in the JDK is needed to add further variables to the config created by jpackage.

[dhruvbajpai29]

Hi @overheadhunter,

Thanks for clarifying that jpackage placeholders (@{userhome}, @{localappdata}) are resolved by the launcher, not the JVM, making them unsuitable for -XX:ErrorFile. I understand a JDK PR to add JVM-compatible variables is complex and beyond #3649’s scope.

To address #3649’s goal of predictable crash log locations, I propose using OS temp directories, which are writable and align with JVM fallback behavior (per Oracle docs). Suggested changes:

• Linux: -XX:ErrorFile=/tmp/Cryptomator/hs_err_pid%p.log
• macOS: -XX:ErrorFile=/tmp/Cryptomator/hs_err_pid%p.log
• Windows: -XX:ErrorFile=%TEMP%\\Cryptomator\\hs_err_pid%p.log

Alternatively, a “never-existing” path (e.g., /nonexistent/hs_err_pid%p.log) could force temp fallback, as @infeo mentioned. I can update the PR with temp paths and retest using sun.misc.Unsafe on Linux to confirm logs appear in /tmp/Cryptomator.

Does this approach work, or are other paths preferred? Thanks for your guidance!

@dhruvbajpai29

[infeo]

I need to correct my self. The documentation says:

By default, this file is created in the current working directory

From my point of view, it is better to always specify the path to a non existent one for deterministic behaviour.

@dhruvbajpai29 Can you update the PR? Please also update the github workflow files (.github/workflows/...)

[coderabbitai]

Walkthrough

Adds a JVM option to jpackage invocations in three GitHub Actions workflows to fix the JVM fatal-error log path. .github/workflows/appimage.yml and .github/workflows/mac-dmg.yml now include -XX:ErrorFile=/cryptomator/cryptomator_crash.log. .github/workflows/win-exe.yml now includes --java-options "-XX:ErrorFile=C:/cryptomator/cryptomator_crash.log". No other jpackage arguments, control flow, or public API declarations were changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• infeo

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues Check	⚠️ Warning	The changes as summarized do add -XX:ErrorFile to packaging steps, but the actual file-level diffs in the raw summary show hard-coded global paths (e.g., /cryptomator/cryptomator_crash.log and C:/cryptomator/cryptomator_crash.log) rather than per-user, app-log-directory locations; this contradicts the linked issue requirements to place crash logs in user-specific app log directories and to avoid requiring elevated permissions. The PR description and objectives describe using runtime placeholders or user-temp locations, but the raw changeset does not reflect that, so the current patch does not meet the primary objectives from issue #3649.	Replace the hard-coded root/global paths with user-local, writable per-OS paths (use jpackage runtime placeholders or per-user temp directories such as @{userhome}/.local/share/Cryptomator/logs or @{userhome}/Library/Logs/Cryptomator and %{LOCALAPPDATA}%\Cryptomator on Windows), ensure the launcher expands placeholders so the JVM receives a valid per-user path, and apply the change consistently in dist/* build scripts and CI/workflow files; re-run packaging tests to confirm hs_err_pid logs land in the expected app log directory.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title succinctly and accurately summarizes the primary change—configuring -XX:ErrorFile so JVM crash logs are written to the app's log directory—and includes the linked issue number for context, making the intent clear to reviewers.
Out of Scope Changes Check	✅ Passed	All modified files in the provided summary are packaging/workflow-related and pertain to setting jpackage JVM options, which is within scope for addressing the linked issue; there are no unrelated codebase changes in the summaries that would indicate out-of-scope edits.
Description Check	✅ Passed	The PR description is directly related to the changeset: it documents adding -XX:ErrorFile to packaging options, explains motivation and implementation decisions, outlines testing performed, and references the linked issue, so it is on-topic and useful for reviewers.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42ff619 and 93bb4c6.

⛔ Files ignored due to path filters (3)

• dist/linux/appimage/build.sh is excluded by !**/dist/**
• dist/mac/dmg/build.sh is excluded by !**/dist/**
• dist/win/build.ps1 is excluded by !**/dist/**

📒 Files selected for processing (3)

• .github/workflows/appimage.yml (1 hunks)
• .github/workflows/mac-dmg.yml (1 hunks)
• .github/workflows/win-exe.yml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• .github/workflows/appimage.yml
• .github/workflows/mac-dmg.yml
• .github/workflows/win-exe.yml

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

.github/workflows/win-exe.yml (1)

169-169: Use forward slashes for Windows path; add timestamp to avoid collisions.

Double backslashes are usually fine, but forward slashes avoid escaping issues and are consistent with other options here. Also consider %t for uniqueness.

-          --java-options "-XX:ErrorFile=C:\\nonexistent\\hs_err_pid%p.log"
+          --java-options "-XX:ErrorFile=C:/nonexistent/hs_err_pid%p_%t.log"

.github/workflows/mac-dmg.yml (1)

139-139: Same as Linux: verify fallback with unwritable path; consider %t.

Please confirm a log is still emitted (and where) on macOS with Temurin 24.0.1+9 when /nonexistent is used. Adding %t helps avoid overwriting in quick successive crashes.

-          --java-options "-XX:ErrorFile=/nonexistent/hs_err_pid%p.log"
+          --java-options "-XX:ErrorFile=/nonexistent/hs_err_pid%p_%t.log"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f64455d and d855538.

⛔ Files ignored due to path filters (3)

• dist/linux/appimage/build.sh is excluded by !**/dist/**
• dist/mac/dmg/build.sh is excluded by !**/dist/**
• dist/win/build.ps1 is excluded by !**/dist/**

📒 Files selected for processing (3)

• .github/workflows/appimage.yml (1 hunks)
• .github/workflows/mac-dmg.yml (1 hunks)
• .github/workflows/win-exe.yml (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-06-17T12:55:22.351Z

Learnt from: infeo
PR: cryptomator/cryptomator#2666
File: .github/workflows/win-exe.yml:0-0
Timestamp: 2025-06-17T12:55:22.351Z
Learning: In the Cryptomator Windows build workflow, the debug launcher (CryptomatorDebug) should be included in every build as an additional, non-standard executable, not conditionally based on the isDebug input. This design provides users with debugging capabilities when needed without affecting the normal user experience.

Applied to files:

• .github/workflows/win-exe.yml

🔇 Additional comments (1)

.github/workflows/appimage.yml (1)

136-136: Confirm fallback on invalid ErrorFile path; optionally include %t suffix across all packaging

– Verify with Temurin 24.0.1+9 that a crash using -XX:ErrorFile=/nonexistent/... still emits an error log (and note its actual output location).
– To avoid name collisions, apply this tweak in:

• .github/workflows/appimage.yml (line 136)
• dist/mac/dmg/build.sh (line 118)
• dist/linux/appimage/build.sh (line 92)
• dist/win/build.ps1 (line 157)

Proposed refactor:

- --java-options "-XX:ErrorFile=/nonexistent/hs_err_pid%p.log"
+ --java-options "-XX:ErrorFile=/nonexistent/hs_err_pid%p_%t.log"

[dhruvbajpai29]

Hi @infeo and @overheadhunter, thanks for the guidance! Updated the pr to use a non-existent path for deterministic behavior. Please review and let me know if any further changes are needed. Thanks

[infeo]

Two things:

1. don't use "nonexistent" as path name. It would be super confusing if this path really exists.
2. Don't call the file hserrorpid. Also here, a it is not clear which process created it. And users need to be able to easily identify the files.

My suggestion: [ROOT]/cryptomator/cryptomator_crash.log
There is only by very low chance a directory in the root called "cryptomator". Even if it exists, the crash log is placed in a related directory. And by naming the file, the user can easily identify it.

[dhruvbajpai29]

Hi @infeo, I've updated the PR based on your feedback, and it's ready for review. I appreciate all your guidance.

[infeo]

Remove the "%p" modifier from the name. This prevents an accumulation of crash logs.

[dhruvbajpai29]

Hi @infeo, my apologies. I've removed the %p modifier as requested and updated the pr.