JDK-8288019 : [cgroups v1] cgroup path logic using substring is dead code in hotspot
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 20
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • OS: linux
  • CPU: generic
  • Submitted: 2022-06-08
  • Updated: 2025-03-10
  • Resolved: 2025-03-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdResolved
Related Reports
Duplicate :  
Description
The code setting the cgroup path vor cgroups v1 reads like this:

/*
 * Set directory to subsystem specific files based
 * on the contents of the mountinfo and cgroup files.
 */
void CgroupV1Controller::set_subsystem_path(char *cgroup_path) {
  char buf[MAXPATHLEN+1];
  if (_root != NULL && cgroup_path != NULL) {
    if (strcmp(_root, "/") == 0) {
      int buflen;
      strncpy(buf, _mount_point, MAXPATHLEN);
      buf[MAXPATHLEN-1] = '\0';
      if (strcmp(cgroup_path,"/") != 0) {
        buflen = strlen(buf);
        if ((buflen + strlen(cgroup_path)) > (MAXPATHLEN-1)) {
          return;
        }
        strncat(buf, cgroup_path, MAXPATHLEN-buflen);
        buf[MAXPATHLEN-1] = '\0';
      }
      _path = os::strdup(buf);
    } else {
      if (strcmp(_root, cgroup_path) == 0) {
        strncpy(buf, _mount_point, MAXPATHLEN);
        buf[MAXPATHLEN-1] = '\0';
        _path = os::strdup(buf);
      } else {
        char *p = strstr(cgroup_path, _root);
        if (p != NULL && p == _root) {
          if (strlen(cgroup_path) > strlen(_root)) {
            int buflen;
            strncpy(buf, _mount_point, MAXPATHLEN);
            buf[MAXPATHLEN-1] = '\0';
            buflen = strlen(buf);
            if ((buflen + strlen(cgroup_path) - strlen(_root)) > (MAXPATHLEN-1)) {
              return;
            }
            strncat(buf, cgroup_path + strlen(_root), MAXPATHLEN-buflen);
            buf[MAXPATHLEN-1] = '\0';
            _path = os::strdup(buf);
          }
        }
      }
    }
  }
}

That is for the substring case, we see a 'p = strstr(cgroup_path, _root)' check that is followed by 'if (p != NULL && p == _root)'. We enter this branch only iff _root != '/' AND cgroup_path != _root. So the only way that the pointer points to '_root' (p == _root) is when cgroup_path is the same pointer than _root, but in that case we should have entered the branch where it is checked that cgroup_path == _root. Thus, this is dead code.

I believe the intention of the original author of this code is to check whether there is a substring match of cgroup_path and _root. If cgroup_path starts with _root, the remainder after _root is being appended to the _mount_path. At least that's what the corresponding Java code does.
Comments
Duplicate of JDK-8343191
10-03-2025

This should no longer happen after JDK-8343191. Closing as duplicate.
10-03-2025

ILW=LHM=P4
26-07-2023

> That is for the substring case, we see a 'p = strstr(cgroup_path, _root)' check that is followed by 'if (p != NULL && p == _root)'. We enter this branch only iff _root != '/' AND cgroup_path != _root. So the only way that the pointer points to '_root' (p == _root) is when cgroup_path is the same pointer than _root, but in that case we should have entered the branch where it is checked that cgroup_path == _root. Thus, this is dead code. There is no branch where this is checked. There is a path where it is checked whether strcmp(cgroup_path, root) holds. The reason that it is dead code is that just because A is a substring of B doesn't mean that A is allocated within B. I flattened the code out and removed the faulty comparison between _root and p. The goal of the code is to set _path to the appropriate value, with some execution flows leading to _path not being set. void CgroupV1Controller::set_subsystem_path(char *cgroup_path) { stringStream ss; if (cgroup_path == nullptr || _root == nullptr) { return; } if (strcmp(_root, "/") == 0) { ss.print_raw(_mount_point); if (strcmp(cgroup_path,"/") != 0) { ss.print_raw(cgroup_path); } _path = os::strdup(ss.base()); return; } if (strcmp(_root, cgroup_path) == 0) { ss.print_raw(_mount_point); _path = os::strdup(ss.base()); return; } char *p = strstr(cgroup_path, _root); if (p == nullptr) { return; } if (strlen(cgroup_path) > strlen(_root)) { ss.print_raw(_mount_point); const char* cg_path_sub = cgroup_path + strlen(_root); ss.print_raw(cg_path_sub); _path = os::strdup(ss.base()); return; } }
09-10-2022

Once JDK-8287007 is pushed this could be verified with the following gtest case: +++ b/test/hotspot/gtest/runtime/test_os_linux_cgroups.cpp @@ -49,9 +49,16 @@ TEST(os_linux_cgroup, set_cgroupv1_subsystem_path) { "/user.slice/user-1000.slice/user@1000.service", // cgroup_path "/sys/fs/cgroup/mem" // expected_path }; - int length = 2; + TestCase substring_match = { + "/sys/fs/cgroup/memory", // mount_path + "/user.slice/user-1000.slice", // root_path + "/user.slice/user-1000.slice/user@1001.service", // cgroup_path + "/sys/fs/cgroup/memory/user@1001.service" // expected_path + }; + int length = 3; TestCase* testCases[] = { &host, - &container_engine }; + &container_engine, + &substring_match }; Which then fails with: [ RUN ] os_linux_cgroup.set_cgroupv1_subsystem_path /disk/openjdk/upstream-sources/git/jdk-jdk/test/hotspot/gtest/runtime/test_os_linux_cgroups.cpp:66: Failure Expected equality of these values: testCases[i]->expected_path Which is: "/sys/fs/cgroup/memory/user@1001.service" ctrl->subsystem_path() Which is: NULL [ FAILED ] os_linux_cgroup.set_cgroupv1_subsystem_path (0 ms)
08-06-2022