JDK-8311285: report some fontconfig related environment variables in hs_err file#14767
JDK-8311285: report some fontconfig related environment variables in hs_err file#14767MBaesken wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back mbaesken! A progress list of the required criteria for merging this PR into |
Webrevs
|
There was a problem hiding this comment.
The XDG values are not specifically fontconfig related.
There was a problem hiding this comment.
This seems a very limited selection - why only these three? (And I find it hard to imagine how these particular ones might lead to crashes - FONTCONFIG_USE_MMAP seems much more interesting in that regard).
|
Hi David , FONTCONFIG_USE_MMAP sounds interesting, I saw this too in the list of environment variables. Should I add this one?
Shared cache home directory (in a shared HOME shared between different hosts) was causing a lot of trouble and leading to crashes rather often in our AIX tests. So this one is interesting for sure. |
It seems more useful, at first glance, than the others :) I find it hard to know where to draw the line here. Over time you could end up eventually adding everything. Would it be better to dump the entire environment to a file along-side the hs_err file? |
|
Hi David,
I am fine with this idea. |
|
Yes I guess dumping the whole environment is a bit too extreme. As it is we have to be cautious about what external information we capture in the hs_err log. |
RealCLanger
left a comment
There was a problem hiding this comment.
This looks like useful additions, I'd also vote for adding FONTCONFIG_USE_MMAP.
|
@MBaesken This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 27 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
tstuefe
left a comment
There was a problem hiding this comment.
LGTM, modulo David's advice (remove the comment, add FONTCONFIG_USE_MMAP).
Cheers, Thomas
|
Hi Thomas and Christoph, thanks for the reviews ! /integrate |
|
Going to push as commit 0ef03f1.
Your commit was automatically rebased without conflicts. |
There are a number of important environment variables influencing how fontconfig works.
See for example
https://man.archlinux.org/man/fonts-conf.5
Some of them should be added to the list of reported environment variables in hs_err file because e.g. a bad setting for some of them can even lead sometimes to crashes.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14767/head:pull/14767$ git checkout pull/14767Update a local copy of the PR:
$ git checkout pull/14767$ git pull https://git.openjdk.org/jdk.git pull/14767/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 14767View PR using the GUI difftool:
$ git pr show -t 14767Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14767.diff
Webrev
Link to Webrev Comment