search

PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.7k stars 2.27k forks source link

Softdepend issues around the 100 plugin mark #5992

Closed JustAMatt closed 1 year ago

JustAMatt commented 3 years ago

Expected behavior

Plugins should respect the softdepends delcared in: https://github.com/Advanced-Kind-MC/ShrineCraft

Observed/Actual behavior

Plugin no longer loads after Custom Items: https://bin.bloom.host/uwoyohoqag.properties Full-Startup log: https://bin.bloom.host/kuqufekoxe.yaml

What leads me to believe that this issue is related to the number of plugins and not the plugins perse is my experience while doing binary testing.

While double-checking if it was caused by another plugin I came to find out 3 suspects which made the plugin no longer work when added to the list, to double-check I removed all other plugins minus essentials and the 3 suspects. And everything worked fine.

Whenever adding more plugins, regardless of the type, the error only shows when the plugin count is above 100.

Steps/models to reproduce

1- Use Paper 1.17-46 2- Have https://cdn.discordapp.com/attachments/737088038622199809/856300216205705236/ShrineCraft-1.1.18.jar <- Which has custom items as a soft depend in plugin.yml 3- Have worldedit 4- Have customitems jar 5- Have more than 100 Plugins 6- Cry as the plugin no longer loads

Plugin list

https://bin.bloom.host/omibamohot.md

Paper version

[18:43:53 INFO]: This server is running Paper version git-Paper-46 (MC: 1.17) (Implementing API version 1.17-R0.1-SNAPSHOT) (Git: 9e07703) You are running the latest version Previous version: git-Paper-"28aacb4" (MC: 1.17)

Agreements

Other

No response

JustAMatt commented 3 years ago

Managed to replicate an issue which causes PlaceholderAPI to break around the same plugin count mark...

electronicboy commented 3 years ago

Question here is if this a soft depend issue, an issue with the graph stuff MD used, or the old fabled yaml bug where it shits itself after a point... This is really something which somebody would need to step through

JustAMatt commented 3 years ago

Question here is if this a soft depend issue, an issue with the graph stuff MD used, or the old fabled yaml bug where it shits itself after a point... This is really something which somebody would need to step through

I'm out of my depth in this regard, but after much testing I can find little difference between the setups except for the Jar used. In this case 1.17 Paper.

JustAMatt commented 3 years ago

I think this issue might become particularly bad seeing how many plugins now require at least 1 external library, or how many plugins require another secondary plugin to implement a specific function for them. So getting to 100 plugins and having so many dependencies within them does not seem out of place.

MartijnMuijsers commented 3 years ago

Just to add: since I moved to 1.17 yesterday with the exact same set of (82) plugins and dependencies, one of the loadbefore's that's completely unrelated to any other dependencies now loads in the wrong order.

Plugin A has loadbefore: [B] Plugin A has no other dependencies listed Plugin B has a number of dependencies but I manually couldn't find anything cyclic (PS. A is my own plugin, and it's just a little thing to do with B, B and others are public plugins so I would assume any cyclic dependencies unlikely anyhow)

(I don't know if before, the loadbefore was being respected or it just happened to always load in the right order - but it always did, since if it doesn't, no-one can join due to a wrong order of execution on join)

Not very rigorous, I know, but if this problem is definitely confirmed, then it might also involve loadbefore, just throwing it out there.

JustAMatt commented 3 years ago

Issue persists with 1.17.1 #88

JustAMatt commented 3 years ago

Does the issue persist for you as well @MartijnMuijsers

MartijnMuijsers commented 3 years ago

Does the issue persist for you as well @MartijnMuijsers

Hi I'm so sorry I've since cut the code from my own plugin A and put it in a different plugin C and added the loadbefore there, now it magically works. I now have the same plugins but 81 instead of 82, and haven't seen a problem in many restarts.

May I recommend trying to open the .jar file of the respective plugins in software like WinRAR, then editing the plugin.yml and replacing it in the jar, and:

Also, verify that

Maybe this will help relieve you from the problem for now, and may also help pinpoint the problem (and whether inherent in Paper or because of something funky with the plugins) better.

JustAMatt commented 3 years ago

In the last 24 hours we made a few key discoveries:

1- The plugins which cause the issue truly seem to be random. 2- It might have something to do with PAPI, when we tested without it we did not observe the issue.

@MartijnMuijsers Could you please test the setup that cause the issue for you (reverting your plugins to the ones that failed) with 1 exception. Remove PAPI and see if it causes the issue or not.

JustAMatt commented 3 years ago

After further testing I'm not 100% sure if it can be blamed on PAPI. I haven't found evidence against it being PAPI, but what could be likely is that the sheer amount of dependencies of PAPI trigger the issue much sooner than what it would otherwise.

Even so I'd still want to see how Martin does.

MartijnMuijsers commented 3 years ago

(I must submit my master's thesis in a week so do not expect a speedy response, apologies)

JustAMatt commented 3 years ago

(I must submit my master's thesis in a week so do not expect a speedy response, apologies)

don't worry :3

IAmSoccer commented 3 years ago

I have this exact same issue, I removed plugins trying to fix it and it eventually works like they said, but it's not 1 certain plugin that makes it happen. It's almost like after so many soft depends are loaded they just stop working or they can't be loaded too fast. So that is my guess to why removing Papi works bc of the amount of soft depends it has are just so many.

mbax commented 3 years ago

To try to eliminate cyclic dependencies from the list of possibilities, I wrote a simple plugin that should find them and output what it found after startup.

https://github.com/KittehOrg/SuperLooperSnooper/releases

Requires Java 16, which shouldn't be an issue at this point. Feel free to build it yourself, decompile, or whatever else makes you comfortable. šŸ˜„

JustAMatt commented 3 years ago

"[23:31:41] [Server thread/ERROR]: [SuperLooperSnooper] Hello! I'm here to snoop for loops! [23:31:41] [Server thread/ERROR]: [SuperLooperSnooper] Found dependency loops: [23:31:41] [Server thread/ERROR]: [SuperLooperSnooper] [PlaceholderAPI, ProNouns] [23:31:41] [Server thread/ERROR]: [SuperLooperSnooper] [CMILib, CMI] [23:31:41] [Server thread/ERROR]: [SuperLooperSnooper] [BanAnnouncer, MaxBansPlus]"

To try to eliminate cyclic dependencies from the list of possibilities, I wrote a simple plugin that should find them and output what it found after startup.

https://github.com/KittehOrg/SuperLooperSnooper/releases

Requires Java 16, which shouldn't be an issue at this point. Feel free to build it yourself, decompile, or whatever else makes you comfortable. šŸ˜„

mbax commented 3 years ago

Loops confirmed:

https://github.com/lucyy-mc/ProNouns/blob/3ced9b11cfdb9617c19c9461b20b873be167246b/pronouns-bukkit/src/main/resources/plugin.yml https://github.com/Spicord/BanAnnouncer/blob/a9847f820941453df5c21e22bdaa93785f33162f/src/main/resources/plugin.yml CMILib confirmed but cannot link to anything because I don't want to deal with the author's hilarious misunderstanding of how Bukkit's GPL license works.

I wonder if removing ProNouns, BanAnnouncer, and CMILib would make you stop experiencing the issue...

lucyydotp commented 3 years ago

I knew that both softdepend and loadbefore was a terrible idea :) thanks for pointing this out, will get it fixed soon

MartijnMuijsers commented 3 years ago

I looked at the source code of the dependency resolution in Bukkit, and it looks correct to me. Perhaps (either I quite a while from now, or someone earlier than that) can submit pull request to Spigot or Paper to print dependency loops.

EDIT: And maybe also rewrite it so it is actually a decent DAG topological sorting, the current implementation is slightly bizarre.

mbax commented 3 years ago

I suspect the reasoning these plugins have is Spigot's decision to publish a warning to the console whenever a plugin touches a class in a way that goes against what the declared dependencies are in plugin.yml.

The biggest issue would be when a plugin wants to loadbefore another plugin but also needs to touch that other plugin's stuff. As there is no uses declaration in plugin.yml to declare intent to interact with another plugin without needing to care about load order, we're sort of stuck getting warnings (or, as I and other authors do, using reflection to modify the dependency graph which feels dirty).

JustAMatt commented 3 years ago

I really hope this is the fix, I would open a ticket with the CMI lad but it's not public šŸ¤·

MartijnMuijsers commented 3 years ago

I really hope this is the fix, I would open a ticket with the CMI lad but it's not public šŸ¤·

(As I noted before, as a temporary fix / test whether it will fix, you can already edit the plugin.yml in the jar file yourself and see)

JustAMatt commented 3 years ago

I really hope this is the fix, I would open a ticket with the CMI lad but it's not public šŸ¤·

(As I noted before, as a temporary fix / test whether it will fix, you can already edit the plugin.yml in the jar file yourself and see)

We are pretty sure in our case the guilty party is pronouns and dev already has the fix in a pr so no need to worry for us atm. We can test it out if it works we could close the ticket from our end but it might be something the paper team might want to fix like it was handled in 1.16 and below.

MartijnMuijsers commented 3 years ago

(Note for PR-er interested people: as for any PRs to the SimplePluginManager, I know Paper has the recent addition of command line plugin loading, but I think PRs for that are better in the Bukkit repo in Spigot)

JustAMatt commented 3 years ago

Update: The change Lucy made to her plugin fixed the issue for us. I'll keep the issue open waiting for the paper team to decide how to approach the changes spigot made in 1.17 that escalated the issue.

Andre601 commented 2 years ago

What is the current state regarding this?

I had two individual users reporting this issue towards me, making me find this. One thing I can add is, that neither person had 100+ plugins. Only below or around 50. One found out that the issue happens while using Slimefun4 (issue linked above).

electronicboy commented 2 years ago

Theres been no progress afaik, and I'm not sure that there is anything for us to do here, not without just introducing another set of potential issues, etc

The # of plugins is generally irrelevant to my understanding, the majority of issues around this are generally irresolvable conflicts between plugins, cyclic dependencies are not generally deal with properly and will generally result in undefined behavior; only real fix I can think of is to ensure that a proper graph is developed, and add many checks to prevent various known issues here, but, such would be a massive diversion away from the behavior of vanilla and cause many large issues for common plugins such as Vault, etc

MartijnMuijsers commented 2 years ago

I think the only thing to do here is add better reporting to a server owner about any cyclic dependency issues. Maybe a new issue for this (just to solidify this is a thing that could be useful)? That would bring to the surface any existing mistakes or edge cases in the implementation too. (Although I do believe the whole current implementation is strange and not ideal and should be replaced altogether: it should just be a topological sort modified as little as possible from the base algorithm - this would immediately give all the things needed to report dependency issues in an understandable way).

Owen1212055 commented 1 year ago

Resolved in https://github.com/PaperMC/Paper/pull/8108