On Mon, 5 Jun 2023 09:02:06 GMT, Aleksandar Pejovic <[email protected]>
wrote:
>> src/java.base/linux/classes/jdk/internal/platform/CgroupInfo.java line 110:
>>
>>> 108: */
>>> 109: static CgroupInfo fromCgroupsLine(String line) {
>>> 110: String[] tokens = line.split("\t");
>>
>> With this change, we now hard-code the expected delimiter and, thus, depend
>> on what the kernel does. Do we have sufficient evidence this hasn't
>> changed/won't change in the future?
>
> As far as I can tell, the delimiter hasn't changed since the file was
> introduced, and judging by the kernel mailing list (e.g., see [the
> following](https://lore.kernel.org/all/[email protected]/)), I
> don't think it will change any time soon.
I'm not convinced this is a good change. Going by that mailing list thread, it
suggests that people considered changing it. If one of those attempts were
successful, it would have broken this code. It makes it rather fragile. The
issue, with container detection code going wrong is that you most likely never
notice. Translating this to GraalVM means that the native image, would a)
detect the wrong version in use or b) fail detection and use host values. In
both cases the application will likely misbehave in a container setup with
resource limits applied and you won't (easily) know why. So even though it's
unlikely to be a problem, there is a chance it could be and it's asking for
trouble for no good reason.
Therefore, being conservative about delimiters makes sense here. My choice in
this case would be more robust code over relying on external factors. YMMV.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14216#discussion_r1217815148
