LuckPerms

LuckPerms

41.4k Downloads

Applying perms does write them to disk but not to cache in rare cases

johnfriedrich opened this issue ยท 21 comments

commented

Luckperms: 4.0.121
SpongeForge: 2889
MC: 1.12.2

Problem:
A plugin of us applies a temp permission when hitting a gui Button. This permission denies the use of that button in the next 23 hours, or at least should.
This only works for the most clicks on that button, but not all.
This used to work with LP Build 61.

Detailed description:
Players can open a gui with a command.
In that gui they will find three buttons. Per button they get a small amount goodies. If they clicked a button it is locked for the next 23 hours. Simple daily rewards.

While opening the gui, three permissions are checked:
pixelmon.dailyreward.normal.cooldown -> Button 1
pixelmon.dailyreward.vip.cooldown -> Button 2
pixelmon.dailyreward.legend.cooldown -> Button 3

If players have lets say pixelmon.dailyreward.normal.cooldown , they cannot click Button 1.

But as it's a new day, they don't have these permissions and are able to click all buttons.
If they click button 1, this command is executed and the menu closes itself
lp user {player} settemp pixelmon.dailyreward.normal.cooldown true 23h
This works as expected. The player now has this perm in his user file.

If he now tries to open the gui again and click on Button 1, he can't click it. as the button checks for that permission.

However, in some cases, this permission won't get saved to cache(but to disk).
Even if the user spamclicks the button and lp sets the temp perm over and over again, it won't save it to cache.
lp sync and player relog does not help. Sometimes the bugs fixes itself after 3 minutes, sometimes after 15 and sometimes never(?)

commented

Addition: The perm gets checked via the sponge method CommandSource#hasPermission.

commented

I don't think this issue is unique to temporary permissions.

One thing worth noting is that commands are executed async. It takes an undefined amount of time for the action to actually take place - although if relogging doesn't help, then it's obviously unrelated to that.

One thing that is odd is that when players re-login, all of their data is totally reloaded. At that point, caches are invalidated.

You can quite easily trace these calls through with an IDE - there's nothing there which could interrupt that process really.

https://github.com/lucko/LuckPerms/blob/master/sponge/src/main/java/me/lucko/luckperms/sponge/listeners/SpongeConnectionListener.java#L84

https://github.com/lucko/LuckPerms/blob/master/common/src/main/java/me/lucko/luckperms/common/utils/AbstractLoginListener.java#L58

https://github.com/lucko/LuckPerms/blob/master/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java#L374

https://github.com/lucko/LuckPerms/blob/master/common/src/main/java/me/lucko/luckperms/common/model/User.java#L195-L200

https://github.com/lucko/LuckPerms/blob/master/common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java#L243

So yea, I'm not really sure what to suggest.

/lp sync pretty much invalidates everything, so if that's not fixing it, then I don't really know. Can you try grab verbose recordings? That would at least confirm that it's a caching issue in LP and not something unrelated (in your plugin for example)

commented

Just tried a simple test, wasn't able to reproduce.

commented

It's very random. Here is a debug. Bit older, but I am not 24/7 online to be around everytime when a user gets that problem.
The button sets the perm on click, but still it's undefined(But I checked the player file at the same time, the nodes are there)
https://gist.github.com/anonymous/2217560a7c2326f146752ce0cda10b7a

commented

Just happened again:
When checking for perm with LP (lp user xxxx permission check xxx.xxx)
http://prntscr.com/ii1s4v
http://prntscr.com/ii1snd

LP debug:
https://gist.github.com/anonymous/4e6b4dbad94fc05e44a00b0ef586ced3

LP sync does not fix that.

But LP says the user already has this perm when trying to set it:
http://prntscr.com/ii1vqg

Conclusion:
It's set in file
setting temp permission says it's already set
lp user permission check perm does return undefined
sponge permission check via plugin does return undefined aswell
lp sync does not fix it.

How does lp user xxx permission check xx.xx.xxx check for nodes? Via sponge? If so, sponge is somehow broken

commented

It checks using the same method.

I still don't really understand the issue.

commented

I've rewritten a lot of the Sponge caching / service implementation - I'm thinking this may resolve this issue.

Can you try updating at let me know if you see any improvements?

commented

Sure, thanks for looking into it

commented

Yes, seeems to be fixed. At least it didn't come up again. Thanks was an annoying and potentially dangerous bug.

commented

Assuming this is fixed?

Feel free to re-open if you're still experiencing the issue.

commented

Pops up again with:
SF: 2990
LP: 4.1.43

Same symptons

Updating to latest atm.

Could that eventually fix it?
c8bb85a

commented

Erm, I don't think so

commented

Yes, but with LP 4.1.58 and SF 2990 I am a bit behind. I will update and report back

commented

Is this still happening regularly?

commented

Still a thing with LP 4.1.80 and SF 7.1 3064

commented

I'm totally lost with this to be honest - not sure what to test etc.

Is there a way for me to easily reproduce the issue?

commented

Updated to this version. Unfortunately there is no way of reproducing this in a quick way. We have hundreds of players using this chestmenu every day and it only happens every two days or so. (Roughly every 700 clicks).
If this update shouldn't solve it, I will try to give you a setup that should make it possible to reproduce it (in a fair time) + code.

commented

I'm still just trying things here... but, the above change may solve it.

It removes the buffer from the reload process, so it happens instantly. May fix this issue

commented

One of the latest changes between 4.1.80 and latest 4.1.93 is causing these watchdog crashes:
https://pastebin.com/4gFub4vw
https://pastebin.com/gCNdz1Sg

commented

Not so far. But seems to be fine. We are for 7 days on a build with that referenced fix so far. Thanks! I will reopen if it should be back for some reasons

commented

Still being problematic?