Skip to content

Fix wrong assert in cgroups code#34291

Merged
jkotas merged 1 commit intodotnet:masterfrom
janvorli:remove-wrong-assert
Mar 30, 2020
Merged

Fix wrong assert in cgroups code#34291
jkotas merged 1 commit intodotnet:masterfrom
janvorli:remove-wrong-assert

Conversation

@janvorli
Copy link
Member

There is an assert in FindCgroupPath that fires when hierarchy_root
and cgroup_path_relative_to_mount are equal, which is the case for
cgroups that are not named. This assert checks that the common
path in those two variables ends with / which is only the case
with named groups.

We have never seen this assert to fire because cgroups initialization
happens before the debugger support initialization in PAL and so
asserts are disabled at that point. I am going to fix that in a
separate PR.

This problem was discovered with the standalone GC where the assert
actually fires as it uses a plain C assert function.

This change fixes the assert to account for the case when both the
paths are the same.

Close #34287

There is an assert in FindCgroupPath that fires when hierarchy_root
and cgroup_path_relative_to_mount are equal, which is the case for
cgroups that are not named. This assert checks that the common
path in those two variables ends with / which is only the case
with named groups.

We have never seen this assert to fire because cgroups initialization
happens before the debugger support initialization in PAL and so
asserts are disabled at that point. I am going to fix that in a
separate PR.

This problem was discovered with the standalone GC where the assert
actually fires as it uses a plain C assert function.

This change fixes the assert to account for the case when both the
paths are the same.
@janvorli janvorli added this to the 5.0 milestone Mar 30, 2020
@janvorli janvorli requested a review from jkotas March 30, 2020 19:52
@janvorli janvorli self-assigned this Mar 30, 2020
@janvorli
Copy link
Member Author

cc: @omajid, @MichalStrehovsky

Copy link
Member

@omajid omajid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an official reviewer, but the fix also makes sense to me 👍

@jkotas jkotas merged commit 162595e into dotnet:master Mar 30, 2020
@jkotas
Copy link
Member

jkotas commented Mar 30, 2020

cc @MichalStrehovsky

@janvorli janvorli deleted the remove-wrong-assert branch March 31, 2020 13:36
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion `cgroup_path_relative_to_mount[common_path_prefix_len] == '/'' failed

3 participants