Permission roles slots sometimes don't exist locally (possibly null parented?) if you are not the host

Nytra commented 7 months ago

Describe the bug?

Edit: This has now been revealed as a bug which causes approx 50% of all hosted sessions of previously saved worlds to have the permissions slots locally missing for non-host users.

Original post:

When using Save As or Save Copy on a world that you are not hosting (e.g. a world hosted by headless), permission roles slots will sometimes be destroyed in the newly saved world.

The primary effect of this bug is that every joining user will have no restrictions on what they can do in that world. It will be like everyone has Admin role.

To Reproduce

1) Create a new Grid world 2) Save the world using Save As (to your own inventory) 3) Edit the metadata for the world so that Anyone can host the world 4) Have a headless client host the world using the world's Record URL while the headless is signed in as another user 5) Join that hosted session and save the world using Save As 6) In the resulting saved world the permission roles will be destroyed.

Expected behavior

The permission roles slots do not get destroyed.

Screenshots

2023-10-30 00 51 24

Resonite Version Number

Beta 2023.12.21.1127

What Platforms does this occur on?

Windows, Linux

What headset if any do you use?

Valve Index, Desktop

Log Files

GRAPHICAL CLIENT - DESKTOP-H976HO2 - 2023.12.21.1127 - 2023-12-25 15_56_49.log

HEADLESS - DESKTOP-H976HO2 - 2023.12.21.1127 - 2023-12-25 16_00_53.log

Additional Context

No response

Reporters

Nytra (Discord: nytra)

code807 commented 7 months ago

LuxKitty, Nytra, and I were able to reproduce this bug in a consistent way with some effort. Here are the steps:

Create a new Grid world
Save the world using Save As (to your own inventory)
Edit the metadata for the world so that Anyone can host the world
Have a headless client host the world using the world's Record URL while signed in as another user
Join that hosted session and save the world using Save As In the resulting saved world the permission roles will be destroyed.

Nytra commented 7 months ago

I have the log from the headless that was hosting the session and there doesn't seem to be anything in there related to permission errors DESKTOP-H976HO2 - HEADLESS HOST - 2023.10.20.831 - 2023-10-30 05_59_10.log

I also looked at code's log and there was nothing.

Nytra commented 5 months ago

I updated the main body of the issue with new log files.

shiftyscales commented 4 months ago

Does this issue only occur with headless hosted sessions, or can it occur with any user-hosted session, @Nytra?

Nytra commented 4 months ago

I have personally only ever seen it happen with headless hosted sessions. I have not tested it with a non-headless hosted session so it cannot be ruled out. @shiftyscales

shiftyscales commented 4 months ago

Okay- if you have time it would be good to check if that is a relevant factor as it'll help narrow down possible causes. Thanks for looking into this, @Nytra.

Nytra commented 4 months ago

Upon further testing I have discovered that it does indeed happen with non-headless users.

My steps to replicate this were as follows:

1) Open two instances of Resonite on the same machine using different database paths for each. (This was only because I didn't have anyone else to test with :P) (You can open one instance via Steam and then another instance by directly clicking on Resonite.exe in the install directory)

2) One instance is logged in as my cloud user, the other as an anonymous user (not logged in)

3) Have the anonymous user open a session of a world owned by my cloud user account (world is able to be opened by anyone via the metadata of the world)

4) Cloud user account joins the session hosted by anonymous user.

5) Cloud user saves copy of the world.

6) Cloud user opens the saved copy of the world and roles slots are destroyed.

I think the key thing here is that a user joins a session of a world that they own that is being hosted by another user, then saves a copy of the world.

Log files:

anonymous user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-10 21_44_36.log

cloud user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-10 21_43_56.log

shiftyscales commented 4 months ago

Thanks for the additional testing!

shiftyscales commented 4 months ago

Can you do some additional testing for me please, @Nytra? In particular, I am wondering if the same bug occurs while using "Save Copy" instead of "Save As". I just ran across #1069 and am wondering if it is somehow related.

Nytra commented 4 months ago

@shiftyscales

Tested with both Save As and Save Copy following the same steps as above with the anonymous user and cloud user. Bug occurred both times.

I'm guessing it's a bug happening when it creates a new record for the world. It would also be interesting to check if the permission role slots are being deleted on saving the world or on loading it.

Save As Logs:

anonymous user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 20_12_19.log

cloud user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 20_11_12.log

Save Copy Logs:

anonymous user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 20_19_20.log

cloud user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 20_18_25.log

shiftyscales commented 4 months ago

Thanks.

Nytra commented 4 months ago

@shiftyscales

Update: I have managed to figure out a way to stop the permission role slots from being lost. It requires the host user to set "AllowChanges" to true in the WorldConfigurationPermissions component. I followed the same steps as above doing a Save As with this and the role slots were not lost.

I think this narrows the issue down to the WorldConfigurationPermissions validation during saving when you are not the host but you own the world record.

2024-01-11 23 25 42

2024-01-11 23 27 21

Save As Log Files w/ AllowChanges=true in WorldConfigurationPermissions:

anonymous user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 23_22_42.log

cloud user - DESKTOP-H976HO2 - 2024.1.3.1265 - 2024-01-11 23_21_48.log

ProbablePrime commented 2 months ago

I sprinkled a hefty sum of debugging and log statements around then used Nytra's non-headless reproduction steps. In comparing my logs to Nytra's I found these interesting.

1:57:59.823 ( 47 FPS) Missing method AddOrRemove (FrooxEngine.ButtonEventHandler`1[FrooxEngine.PermissionSet] on Element: ID10CB900, Type: FrooxEngine.UnresolvedReferences+UnresolvedReference,

UnresolvedReferences is a component that get's added when a Reference can't be resolved during loading a world/item. These usually should not happen unless something changes between saving or loading(like component renames) or information is missing from the "save"

This suggests that either saving the permissions settings or loading them is where the issue is.

None of the log lines I added in the permission system(WorldConfigurationPermissions included) seemed to trigger and none of the pre-existing ones triggered also.

ProbablePrime commented 2 months ago

With some help from Cyro, I took a copy of the world from Step 5 of Nytra's Replication steps with my own world(So this is a copy that the owner has saved from the anonymous user who was hosting it) and downloaded its raw bson. Examining this in its raw JSON form leads to finding that the permission slots are already missing in the save.

So this means it is in the saving/serialization of the world. For some reason the permission slots are being deleted or ignored in saving.

This is helpful as it means I can focus on the Save process of a world.

ProbablePrime commented 2 months ago

Here's an example in the raw JSON.

There's only one reference to "GrabablePermissions" and it is from the inspector that's in the world. There should be at least one GrabbablePermissions component in the save but there is not.

ProbablePrime commented 2 months ago

I full read and debugged the permissions system yesterday. As an admin in the world you'll never hit basically any permission checks(Admins are only subject to LocomotionPermissions). So that was a red herring.

Additionally, there are two separate parts of a world where permissions are stored:

The permissions system itself, which is an object outside of the world hierarchy
The slots that we're seeing missing.

The fact that the slots are missing is somewhat easier to trace because something must be either "ignoring" them or "deleting" them, there's no other way for a slot to vanish like that.

Looking into the saving operation of a world I just logged every slot as it went through the save:

03:28:11.242 (132 FPS) Roles is being saved 03:28:11.243 (132 FPS) Non-Admin World Configuration Permissions is being considered for saving 03:28:11.243 (132 FPS) Roles has child Fart 03:28:11.243 (132 FPS) Roles is being saved 03:28:11.243 (132 FPS) Fart is being considered for saving 03:28:11.243 (132 FPS) Root has child Clipboard Importer

When we get here, it isn't even finding the other roles slots in the save process.

All permission components have some extra flags enabled on their values called "HostOnly" and "DirectAccessOnly" but WorldConfigurationPermissions is included in this.

The only materialistic difference between WorldConfigurationPermissions and the rest is that it acts as an instance validator and NOT a global validator. This means it just validates one instance of an object and not all instances of a type.

Oh and Fart in the above output is an extra slot I added to the roles hierarchy to see if it was preserved and it is.

I'll continue digging.

Nytra commented 2 months ago

It might be worth looking for places where it checks if the user is the host or is authority

Nytra commented 2 months ago

The reason why I thought it was WorldConfigurationPermissions is because if you tick AllowChanges the roles are kept, and the Validate method uses the result from CanUserChange:

public bool CanUserChange(User user)
{
    if (!AllowChanges)
    {
        return user.IsHost;
    }
    return true;
}

It checks for host here which would be false in this case.

ProbablePrime commented 2 months ago

I made some changes so when a world is saved right at the last opportunity during world saving it dumps the entire world in a save into a file. Then I replicated the above bug and checked the dump.

p0qa3et1fc5ud9iu.json

The dump HAS the roles.

So perhaps cloud processing is at fault here.

I'll look through the cloud saving of it.

ProbablePrime commented 2 months ago

Looked through the entire stack for saving records to the cloud, found nothing wrong
Dumped the world locally again because I ran out of things to check, the roles were missing
Took a nap because I must be going mad
Roles were missing at same point again... yay
So we're back to client side stuff.

I hope everyone appreciates the updates, see we're doing a ton of stuff behind the scenes. Including saving a world over and over again while making screaming noises!

Nytra commented 2 months ago

I did earlier try doing the bug but I saved the world to my local machine using "Save Here" on the orb while focused in local home and the permissions slots were not lost

lxw404 commented 2 months ago

I hope everyone appreciates the updates, see we're doing a ton of stuff behind the scenes. Including saving a world over and over again while making screaming noises!

We definitely appreciate these, I can feel the suffering vicariously.

ProbablePrime commented 2 months ago

Ok after more debugging this is nothing to do with saving.

I have achieved repeated tests of a log line that logs the number of slots under "Roles". It is 1 all of the time. Even in the button event handler for the save copy button itself.

I'll look in other areas.

ProbablePrime commented 2 months ago

Oh hey I have a work around!!!!

In @Nytra 's original replication steps if you in between 4 and 5 add another step, we'll call it 4A.

4A. Cloud user or Host must open an inspector and expand the "Roles" slot in the inspector.

With 4A added the world saves correctly.

EDIT: It also explains why I lost so much time last week thinking it was saving or loading. In some tests I had the roles slot open to check they were still there between tests. On the tests where i did not double check the roles existed before saving they didn't save and on the ones where I did they DID save. This is what lead to the confusion.

This bug feels like lord of the rings. We're on to Part 3, If a tree falls in the woods and no one is around does it make a sound?

ProbablePrime commented 2 months ago

Ok, 4A no longer works, I was tripple replicating it before I added it to the wiki and it stopped working. I'm going slightly insane but done for the day.

Nytra commented 2 months ago

Thank you for working on this

art0007i commented 2 months ago

Ok after more debugging this is nothing to do with saving.

I have achieved repeated tests of a log line that logs the number of slots under "Roles". It is 1 all of the time. Even in the button event handler for the save copy button itself.

I'll look in other areas.

I've seen something like this before, I think that the user who saves the world isn't even aware of the permissions in the first place. This can be confirmed by adding a FindChildByName node and searching for "Guest Permissions" and they will show up for the host, but not for the user who is about to save the bugged world.

Nytra commented 2 months ago

Ok after more debugging this is nothing to do with saving. I have achieved repeated tests of a log line that logs the number of slots under "Roles". It is 1 all of the time. Even in the button event handler for the save copy button itself. I'll look in other areas.

I've seen something like this before, I think that the user who saves the world isn't even aware of the permissions in the first place. This can be confirmed by adding a FindChildByName node and searching for "Guest Permissions" and they will show up for the host, but not for the user who is about to save the bugged world.

Omg you are right. That's crazy.

ohzee00 commented 2 months ago

Wait, how does that make sense? Inspectors are generated by the host, how could they hide something the host can see but not the client?

Nytra commented 2 months ago

I am testing it using some basic flux, it appears the permission roles slots literally don't exist for non-host users somehow

And if you try to make a reference to them it will say "Parent: Root" instead of "Parent: Roles"

I am testing on headless

ohzee00 commented 2 months ago

Does it work in the world that you host yourself..? Or only on a headless hosting it? I tried testing myself on a world that had its perms wiped (both on a headless AND hosting it myself) due to the issue here, however I could find nothing though this has been resaved multiple times in this state. It's very possible these weird.. phantom permissions don't last through another save

Nytra commented 2 months ago

I am testing on headless

Nytra commented 2 months ago

~In my case testing on headless, this does seem to be linked to https://github.com/Yellow-Dog-Man/Resonite-Issues/issues/490~

~In cases where two UndoManagers were created in the world, the Roles slots were normal~

~In cases where it did not make two UndoManagers, the Roles slots were bugged~

Continued testing revealed it does not always coincide with the two UndoManager bug

Nytra commented 2 months ago

It does happen for non-headless too. (The thing to look for here is the Parent reference being different)

2024-03-11 11 39 38

But it's inconsistent and probably a race condition same as for the UndoManager thing

Nytra commented 2 months ago

It doesn't even need to be a world that you own the record of, it seems it can happen for any world. (The thing to look for here is the Parent reference being different)

2024-03-11 12 19 29

Nytra commented 2 months ago

Updated issue title to be more descriptive of the actual issue

lxw404 commented 2 months ago

So in summary from testing this with Nytra, what we've found so far is:

This happens completely randomly
Causes a local parenting of all of the children in the Roles slot for everyone but the host to be Root
When that non-host user who has the ability to save the world saves while in this state, the permissions are lost

lxw404 commented 2 months ago

Just to consolidate some testing into two groups:

The first was hosting a world that was saved. By random chance, the non-host immediately notices that the Roles slots are parented locally to Root:

Roles Destruction Testing (Host) -2024.3.1.1178-_2024-03-11_13_05_24.log

Roles Destruction Testing (Non Host) - 2024.3.1.1178 - 2024-03-11 09_05_22.log

The second, we repeatedly opened saved blank default grid worlds until the issue happened.

Roles Destruction Testing 2 (Eventually Host) -2024.3.1.1178-_2024-03-11_13_10_14.log

Roles Destruction Testing 2 (Fail on non host) - 2024.3.1.1178 - 2024-03-11 09_09_54.log

Nytra commented 2 months ago

We have made a tool you can find in this folder: resrec:///G-Bright-Spark/R-89C8164D926CE5E41E6DBDD5C912FD9E56F8FDFEAFBE0D1192A22D4893258D79

Spawning it in a world with multiple users will tell you if the (default) permissions slots are missing for any users and it will do the amongus

2024-03-11 14 18 06

art0007i commented 2 months ago

I think this has something to do with the system that is responsible for making sure you can't change the parent of the roles slot or the individual role slots.

I've created a mod that makes the Slot.MarkProtected method do nothing and this seems to prevent the bug from happening.

Nytra commented 2 months ago

I also have not had any luck getting the bug to happen for worlds freshly created from template that haven't been saved before

lxw404 commented 2 months ago

Nytra has automated the process a bit and allowed us to collect some limited data:

We visited randomized featured worlds which have their permissions in tact on the host's side.

50.0% of the time (10 out of 20 unique sessions) the sus emerged, as visible by the non-host
Every unique world we have visited has exhibited this issue at least once

We tested the individual generated template worlds (new never before saved) many times and never got a single one to exhibit the issue.

ProbablePrime commented 2 months ago

I go to bed and wake up with all of this help.

THANK YOU everyone.

Lemme make some notes here.

@lxw404 and @Nytra thank you for your help in determining that this is somewhat "random". It's been driving me mad and now we know why. I also love the tool. Should save some grief. We've come a long way from "Permissions are being deleted" to "Permissions are not sent by the host"

@art0007i Yep you're onto something, I noticed it last night but forgot to comment and even added a temporary thing to prove it.

As previously mentioned above, there are two permission "validators" types:

Instance Change Validators - Validate a single instance of a component/object
Global Change Validators - Validate all instances of a component/object by type.

The components in the "Non-Admin World Configuration Permissions" slot only use Instance validators and NOT global validators.

Examining some of the code, there's a subtle difference in the validator's use of "MarkProtected", that occurs for each validator and the way it is configured.

For Global Change Validators we "MarkProtected" the slot that the Global Change Validator exists on, In this case the slot that holds things like "Grabbable Permissions" etc.
For Instance Change Validators we just "MarkProtected" the slot that the Instance we're validating is on.

So in other words:

All permission slots except for the "Non-Admin World Configuration Permissions" slot are "MarkProtected"
Non-Admin World Configuration Permissions is NOT "MarkProtected"

So the change I made last night in a half asleep fugue was to edit the code for the Instance validator to also mark the slot that the Instance Validator is on as Protected. This is probably a good idea generally. But doing this should then cause ALL permission slots to be destroyed with this bug rather than leaving behind. "Non-Admin World Configuration Permissions"

I'll spend time today looking into exactly what "MarkProtected" does.

EDIT: Oh the UndoManager is not markprotected!

ProbablePrime commented 2 months ago

Something tells me I'm onto something:

ProbablePrime commented 2 months ago

Got it. To explain what you're seeing in this image, there's two logging lines.

runs when a new parent is being given to a slot. It details where that new parent is and what caused it with the stack trace.
Runs when a slot is protected during the parenting operation it was written by me while cackling so is poorly formatted and confusing, ignore it except for it existing which means that the protection logic is activated.

The highlighted section in the middle is part of the stack trace and is explaining that the new parent for the slot "non-builder permissions" has come from LNL.

In this case its coming from the "Syncing initial world state" part of loading a world. So Resonite's trying to build the session from the full state of the world sent over LNL. When doing this is of course needs to make slots and parent them but the roles slot in question is protected. So it gets set back to null.

The protections of a slot should only apply after the initial parent is set. In this case "currentParent: ()" it has not received an initial parent so its parent is null.

There's a bunch of logic here in the parenting steps that I don't quite understand at the moment, so I've gotta noodle on it some more.

BTW there's some indication here that this behavior is repeating(the logs are really big), so fixing this bug might have a slight performance boost in cases where you're working in a world that has previously had its roles "destroyed"

ProbablePrime commented 2 months ago

:)

Oh and the log file from the last test of this before I opened the PR is 300 MB of the protected slot trying to re-parent to the correct location :D

ProbablePrime commented 2 months ago

Oh and to explain the "randomness":

If the parent of the slot was set before the slot was protected it would work. If the parent of the slot was not set before the slot was protected it wouldn't work and we'd see the bug.

So in some cases the parenting would be ok and in other cases it wouldn't....randomly... based on the order of operations of protecting and parenting which I think based on this could be non-deterministic(the world state is being sent over UDP) but im not sure..

Frooxius commented 2 months ago

Merged fix in 2024.3.12.1169, thank you!

Yellow-Dog-Man / Resonite-Issues