On Thu, 12 May 2022 18:09:40 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:

>> Please review this change to the cgroup v1 subsystem which makes it more 
>> resilient on some of the stranger systems. Unfortunately, I wasn't able to 
>> re-create a similar system as the reporter. The idea of using the longest 
>> substring match for the cgroupv1 file paths was based on the fact that on 
>> systemd systems processes run in separate scopes and the maven forked test 
>> runner might exhibit this property. For that it makes sense to use the 
>> common ancestor path. Nothing changes in the common cases where the 
>> `cgroup_path` matches `_root` and when the `_root` is `/` (container the 
>> former, host system the latter).
>> 
>> In addition, the code paths which are susceptible to throw NPE have been 
>> hardened to catch those situations. Should it happen in the future it makes 
>> more sense (to me) to not have accurate container detection in favor of 
>> continuing to keep running.
>> 
>> Finally, with the added unit-tests a bug was uncovered on the "substring" 
>> match case of cgroup paths in hotspot. `p` returned from `strstr` can never 
>> point to `_root` as it's used as the "needle" to find in "haystack" 
>> `cgroup_path` (not the other way round).
>> 
>> Testing:
>> - [x] Added unit tests
>> - [x] GHA
>> - [x] Container tests on cgroups v1 Linux. Continue to pass
>
> Severin Gehwolf has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Use stringStream to simplify controller path assembly

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 92:

> 90:           }
> 91:           ss.print_raw(_root, last_matching_slash_pos);
> 92:           _path = os::strdup(ss.base());

Do you mean `Find the longest common prefix`? Maybe give an example in the 
comments? Text parsing in C code is really difficult to understand.

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

PR: https://git.openjdk.java.net/jdk/pull/8629

Reply via email to