[crac] RFR: 8378446: [CRaC] Replace libcrexec with libcriuengine [v3]

[Timofei Pushkin]

On Thu, 26 Feb 2026 13:25:34 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> After [JDK-8376959](https://bugs.openjdk.org/browse/JDK-8376959) the only use of libcrexec is running CRIU; in fact the code already is quite CRIU-dependent. The point of this task is to stop pretending that libcrexec is generic, and move code from criuengine binary (now removed) into libcriuengine implementing the C/R API.
>> 
>> This removes smuggling of some parameters through environment variables and execution of the criuengine. We still require anexecuteable (now called `criuhelper`) to become parent of the restored process, but this has significantly simplified implementation.
>> 
>> Replacement of communication between the restoring and restored process through signals and temporary files is out of scope of this change.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments

LGTM overall, there's just one place where I believe a bit of memory may leak (+ some stylistic stuff which I am OK to ignore)

make/modules/java.base/Lib.gmk line 220:

> 218:   CXXFLAGS_macosx := -DMACOSX, \
> 219:   LDFLAGS := $(LDFLAGS_JDKLIB), \
> 220: ))

Indentation of arguments should've stayed 4 — this is how it is in the rest of the file. AFAIU, it is 2 for statements, 4 for for mid-statement line breaks like in this case.

But this should have no functional implications.

src/java.base/linux/native/libcriuengine/criuengine.cpp line 109:

> 107: }
> 108: 
> 109: class ArgsBuilder {

The class could've been more robust:
- Non-copyable (or with proper copying implemented)
- Initialize `_args[0]` to null, or just the whole array instead of managing last-is-null dynamically
- Assert that `append_all` is not used multiple times (`_from_env` and `_from_opts` are null)
- Assert that no methods are called when `_failed` is true

But since, as far as I can see, currently it is used in a way that the above won't fail I don't insist.

src/java.base/linux/native/libcriuengine/criuengine.cpp line 119:

> 117:   void fill_args(const char* from, char* copy) {
> 118:     char* saveptr = nullptr;
> 119:     char* arg = strtok_r(copy, " ", &saveptr);

Strange to have both `char*` and `char *` in the same class

src/java.base/linux/native/libcriuengine/criuengine.cpp line 502:

> 500: static const char *resolve(const char *rel, char resolved[PATH_MAX]) {
> 501:     char *abs = realpath(rel, resolved);
> 502:     return abs ? abs : rel;

Suggestion:

  char *abs = realpath(rel, resolved);
  return abs ? abs : rel;

src/java.base/linux/native/libcriuengine/criuengine.cpp line 678:

> 676: 
> 677:   // We need to use ../lib for static build
> 678:   // This buffer is not free'd but we don't care since we'll execve

But if we fail before exec completes and `-XX:+CRaCIgnoreRestoreIfUnavailable` is set I believe the JVM will proceed with this memory lost

-------------

PR Review: https://git.openjdk.org/crac/pull/297#pullrequestreview-3865343981
PR Review Comment: https://git.openjdk.org/crac/pull/297#discussion_r2862883894
PR Review Comment: https://git.openjdk.org/crac/pull/297#discussion_r2863009376
PR Review Comment: https://git.openjdk.org/crac/pull/297#discussion_r2862922404
PR Review Comment: https://git.openjdk.org/crac/pull/297#discussion_r2863034693
PR Review Comment: https://git.openjdk.org/crac/pull/297#discussion_r2863056520