PermissionManager: fixed non-reentrant-safe permission unsubscribing

during unset(), the destructors for other objects with cyclic references can get triggered, resulting in the functions being reentered before the count() call. This leads to a crash because the offset no longer exists.
Instead, we check if only the given PermissibleInternal is present, and clean everything up with a single unset instead of two.
This could also have been solved by adding extra isset() checks before checking the counts, but this way seemed more elegant.

This is similar to an issue with AsyncTask thread-local storage a few months ago, which was also caused by GC reentrancy.

closes #6119
This commit is contained in:
Dylan K. Taylor 2023-11-01 16:13:28 +00:00
parent 0b2fc84827
commit 0093732d49
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D

View File

@ -72,19 +72,21 @@ class PermissionManager{
}
public function unsubscribeFromPermission(string $permission, PermissibleInternal $permissible) : void{
if(isset($this->permSubs[$permission])){
unset($this->permSubs[$permission][spl_object_id($permissible)]);
if(count($this->permSubs[$permission]) === 0){
if(isset($this->permSubs[$permission][spl_object_id($permissible)])){
if(count($this->permSubs[$permission]) === 1){
unset($this->permSubs[$permission]);
}else{
unset($this->permSubs[$permission][spl_object_id($permissible)]);
}
}
}
public function unsubscribeFromAllPermissions(PermissibleInternal $permissible) : void{
foreach($this->permSubs as $permission => $subs){
unset($this->permSubs[$permission][spl_object_id($permissible)]);
if(count($this->permSubs[$permission]) === 0){
if(count($subs) === 1 && isset($subs[spl_object_id($permissible)])){
unset($this->permSubs[$permission]);
}else{
unset($this->permSubs[$permission][spl_object_id($permissible)]);
}
}
}