Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a ClassLoader stop bypass for SuperiorSkyblock2 plugin in addition of current SlimeWorldPlugin #135

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

GrayR0ot
Copy link

@GrayR0ot GrayR0ot commented Sep 7, 2024

SInce 1.20.3 (if I'm not wrong) bukkit unloads plugin ClassLoader on the "onDisable" phase of a plugin.

But the fact is that some plugins are saving data during this period.
A good example is the module "loaders" of ASP which is doing it asynchrously.
I found this commit which was the same fix for ASP plugin:

https://github.com/InfernalSuite/AdvancedSlimePaper/blob/main/patches/api/0002-Dont-close-Slime-Plugin-Classloader.patch

The current issue this PR should fix is the following:

[20:56:11 INFO]: [SuperiorSkyblock2] Disabling SuperiorSkyblock2 v2024.3
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module bank...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module missions...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module SSB-MongoDB...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module generators...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module upgrades...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module SlimeWorldModule...
[20:56:11 INFO]: Saving world island_00829379-d880-4a71-8345-f4d2f68b563c_normal...
[20:56:11 WARN]: java.lang.IllegalStateException: zip file closed
[20:56:11 WARN]: 	at java.base/java.util.zip.ZipFile.ensureOpen(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.zip.ZipFile.getEntry(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.jar.JarFile.getEntry(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.jar.JarFile.getJarEntry(Unknown Source)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:209)
[20:56:11 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass0(PluginClassLoader.java:169)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass(PluginClassLoader.java:164)
[20:56:11 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[20:56:11 WARN]: 	at gens.jar//libs.com.mongodb.client.gridfs.GridFSBucketImpl.uploadFromStream(GridFSBucketImpl.java:189)
[20:56:11 WARN]: 	at gens.jar//libs.com.infernalsuite.aswm.loaders.mongo.MongoLoader.saveWorld(MongoLoader.java:152)
[20:56:11 WARN]: 	at com.infernalsuite.aswm.level.SlimeLevelInstance.lambda$saveInternal$1(SlimeLevelInstance.java:160)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.lang.Thread.run(Unknown Source)
[20:56:11 INFO]: [SuperiorSkyblock2-SlimeWorldModule] Unloaded island world island_00829379-d880-4a71-8345-f4d2f68b563c_normal in 19ms
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down calculation task...
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down executor
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down database executor
[20:56:12 INFO]: [SuperiorSkyblock2] Closing database. This may hang the server. Do not shut it down, or data may get lost.

If this PR is not correct, ofc I can do changes/delete it. But we can also talk about it

Paul19988 and others added 12 commits August 11, 2024 21:15
…3a454545ab

[ci skip] Fix JavaDocs for HeightMap#MOTION_BLOCKING_NO_LEAVES (#11291)

Automatically updated to Paper commit: ba1b0162faaa7bf78d33162f6976586bb3049275
Allow server administrators to disable book size checks (#10457)

Automatically updated to Paper commit: dae906ba452e4a17779aa6438ccd6deb42d7f0df
Add getWorld method that uses adventure Key (#11199)

Automatically updated to Paper commit: 8fd3a67138b5d7a3a21e059eb3c3b766ffdfc281
[ci skip] Clean up book limits patch (#11297)

Automatically updated to Paper commit: be1078f5f2e28c1a1de99e9c96e4003c3f96f6e8
Use player file, not directory, when checking for offline player data

When trying to fall back to offline player data in onlide mode,
we need to use the player file. This fixes a mistake during
update where 'file' was used, but the new code uses 'file1'
for the player file.

Automatically updated to Paper commit: b4bc512cac4d75f9e366479c5d8fdcf4275c85ce
Fix entity limit patch deleting unnecessary entities

We need to continue the save loop, not break from it
when a limit is reached.

Automatically updated to Paper commit: 64c9ee6584ce15e5b486bdc7183fe902e22fddf3
Allow getting/setting the sign's editor uuid (#10637)

* Allow getting/setting the sign's editor uuid

* rebased

---------

Co-authored-by: Jake Potrebic <[email protected]>

Automatically updated to Paper commit: 44017487a81534067fd17e2d5e8669e61e19d719
Fix CraftBukkit drag system (#10703)

Automatically updated to Paper commit: 8c3018a4b177aa0f7658d510bf7d19c5fa8e5b65
Fix Selector Arguments not working with permission (#11286)

Automatically updated to Paper commit: 57dd822393df78a4cb47f7a2c2d037dc2a77d5bd
Leashable API (#10961)

Automatically updated to Paper commit: 2e82fd2d2c1881665e27654977390fa48f5ec346
Add even more Enchantment API (#11115)

Automatically updated to Paper commit: 66a97cc929740475bc8dd192382c4ae59fdeace9
Update item data sanitization (#11227)

Automatically updated to Paper commit: 0e7361704a16c8386d829c26603f3e5c0bf72b39
Updated Upstream (Bukkit/CraftBukkit/Spigot) (#11284)

Updated Upstream (Bukkit/CraftBukkit/Spigot)

Upstream has released updates that appear to apply and compile correctly.
This update has not been tested by PaperMC and as with ANY update, please do your own testing

Bukkit Changes:
4068c6aa PR-1053: Change docs for max power in FireworkMeta
6b3c241b SPIGOT-7783, SPIGOT-7784, PR-1051: Add Trial Vault & Spawner event API
5fe300ec PR-1052: Fix broken links and minor improvement for checkstyle.xml

CraftBukkit Changes:
7548afcf2 SPIGOT-7872: Fix crash with event-modified teleports
93480d5d6 SPIGOT-7868, PR-1463: Fix default and max power in FireworkMeta
5060d1a84 SPIGOT-7783, SPIGOT-7784, PR-1460: Add Trial Vault & Spawner event API
11dfcae71 PR-1462: Fix broken links and minor improvement for checkstyle.xml

Spigot Changes:
ca581228 Rebuild patches

Automatically updated to Paper commit: 75655ec1d34f344c7b0ba9f31f2ada87951d277d
Add Configuration for vertical Despawn Ranges (#11279)

Automatically updated to Paper commit: 52ae4ad4666b34d637f2de573ed03c02b0fc6a24
Migrate ArmorStand meta to using entity tag (#11107)

Automatically updated to Paper commit: d5ffc573dc17092ea2cfdd4ce14dac3e70b932f5
Implement more methods for horse inventories (#11147)

Automatically updated to Paper commit: 4829fbf6bd51e2dc86e2f190971c081aac76a201
Handle custom registry elements properly (#11230)

* Handle custom registry elements properly

* update error message

Datapack made painting variant support is added in PaperMC/Paper#11244

* change msg for art conversion

---------

Co-authored-by: Jake Potrebic <[email protected]>

Automatically updated to Paper commit: 78216fef26e454c78dff5e495f6ebe673be56900
Re-implement portalCreateRadius world config (#11267)

Automatically updated to Paper commit: e619744fbdc4963eed0b04c1e44bd1089ce50285
Allow skipping of world symlink validation (#11250)

Automatically updated to Paper commit: 534ab86010330ce3afa732910227585d0dc02d1e
[ci-skip] Revert "Add Configuration for vertical Despawn Ranges (#10440)" (#11278)

This reverts commit 1b8ab116edd5da15791de96aa462db90756848dc.

Automatically updated to Paper commit: 1b8ab116edd5da15791de96aa462db90756848dc
Add Configuration for vertical Despawn Ranges (#10440)

Automatically updated to Paper commit: ec55c11fc074929e4aa1a1ecdaef51da69dbf0be
Fix indestructable light blocks (#11275)

Co-authored-by: Bjarne Koll <[email protected]>

Automatically updated to Paper commit: 95719832bf57ae523982d681cd219e8387c39955
Fix scanForLegacyEnderDragon world config (#11262)

Automatically updated to Paper commit: 81bfda87103e1a47ea184c5da119e79218a8dc1e
[ci skip] Specify rebase location in CONTRIBUTING (#11255)

* [ci skip] Specify rebase location in CONTRIBUTING

* Improve

* remove Paper-MojangAPI mention

---------

Co-authored-by: Bjarne Koll <[email protected]>
Co-authored-by: Lulu13022002 <[email protected]>

Automatically updated to Paper commit: fb530743e5c71cb4faba06339bdeb14a33e06a6a
Apply optimise collision checking in move packet handling patch

Automatically updated to Paper commit: bf5852a6151aa643b87bd0e0f9e3940f867064a3
Fix NPE for PlayerPostRespawnEvent#getRespawnedLocation (#11268)

Automatically updated to Paper commit: 11b4ac7c659ffb7e4790b0856b14117b6d532200
Fix disableEndCredits world config (#11261)

Automatically updated to Paper commit: 9ab644ed290542a9e537d8f529bc6898a3da2e7d
Fix `TooltipContext.create` being wrong(#11254)

Co-authored-by: Jake Potrebic <[email protected]>

Automatically updated to Paper commit: 7c9240f4a63b13be1fdcedbfb0270f9b49b75518
Improve standard messenger logging

Automatically updated to Paper commit: 1798e949e5727f376ccaee51873f21dcdedc9a5f
Fix BasicCommand suggestion arg count (#11241)

Automatically updated to Paper commit: 4a97ba3ea8cb449fe76ed1aef0b572e7cc01d542
Fix `setSendViewDistance`'s return (#11247)

Automatically updated to Paper commit: f97aff74b6f7fd4940c0c0b6cca3f8ac6e1afdef
[ci skip] Fix Effect javadocs (#11182)

Automatically updated to Paper commit: 098bd39092f50cb97a604c2f228f2af29666ca41
Remove arbitrary book page limit (#11228)

Automatically updated to Paper commit: ab0d24aa6f22308352eb363b09c992fc70de4143
Configuration for horizontal-only item merging (#11219)
Migrate back to the gradleup shadow plugin and upgrade version to 8.3.0
Fix typo: worldData.name() was added twice to WorldsInUse instead of being removed.
Fixes the issue with chunks not saving sometimes when unloading the world
@GrayR0ot GrayR0ot marked this pull request as ready for review September 7, 2024 21:19
@CommandDan
Copy link

I don't think fixes for SSB2 should be in ASP. Their fixes can be done in a fork specifically for them.

ASP should in my opinion only contain fixes, features, etc. related to ASP in general for everyone.

@GrayR0ot
Copy link
Author

GrayR0ot commented Sep 7, 2024

In a way I'm agree with you. I added a public static final List for it. Maybe it could allow plugins to add them self to this bypass list?

But to be honest a huge part of ASP users are using Superior Skyblock, and this fix is really small. It does not break anything else. This is only a "bad" practice to fix it easier.

@Leguan16
Copy link

Leguan16 commented Sep 8, 2024

I think a general setting of "keep my plugin excluded from this" would be nice. I'm sure there are other plugins that could also benefit from it.

@GrayR0ot
Copy link
Author

GrayR0ot commented Sep 8, 2024

Very good idea

@b0ykoe
Copy link
Contributor

b0ykoe commented Sep 9, 2024

In my opinion you either should also expand the api to actually be able to properly add a plugin to the excluding list or you should make it a paper/bukkit or whatever config value. Having that one hardcoded and undocumented seems like a bandage aid fix for SSB2.

@GrayR0ot
Copy link
Author

Yes this is purely a proof of work bypass. We are looking for the best solution. Implementing a fix within the new V3 API might be the best one.

@kyngs
Copy link
Member

kyngs commented Sep 10, 2024

SInce 1.20.3 (if I'm not wrong) bukkit unloads plugin ClassLoader on the "onDisable" phase of a plugin.

But the fact is that some plugins are saving data during this period. A good example is the module "loaders" of ASP which is doing it asynchrously. I found this commit which was the same fix for ASP plugin:

https://github.com/InfernalSuite/AdvancedSlimePaper/blob/main/patches/api/0002-Dont-close-Slime-Plugin-Classloader.patch

The current issue this PR should fix is the following:

[20:56:11 INFO]: [SuperiorSkyblock2] Disabling SuperiorSkyblock2 v2024.3
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module bank...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module missions...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module SSB-MongoDB...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module generators...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module upgrades...
[20:56:11 INFO]: [SuperiorSkyblock2] Disabling the module SlimeWorldModule...
[20:56:11 INFO]: Saving world island_00829379-d880-4a71-8345-f4d2f68b563c_normal...
[20:56:11 WARN]: java.lang.IllegalStateException: zip file closed
[20:56:11 WARN]: 	at java.base/java.util.zip.ZipFile.ensureOpen(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.zip.ZipFile.getEntry(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.jar.JarFile.getEntry(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.jar.JarFile.getJarEntry(Unknown Source)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:209)
[20:56:11 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass0(PluginClassLoader.java:169)
[20:56:11 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass(PluginClassLoader.java:164)
[20:56:11 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[20:56:11 WARN]: 	at gens.jar//libs.com.mongodb.client.gridfs.GridFSBucketImpl.uploadFromStream(GridFSBucketImpl.java:189)
[20:56:11 WARN]: 	at gens.jar//libs.com.infernalsuite.aswm.loaders.mongo.MongoLoader.saveWorld(MongoLoader.java:152)
[20:56:11 WARN]: 	at com.infernalsuite.aswm.level.SlimeLevelInstance.lambda$saveInternal$1(SlimeLevelInstance.java:160)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[20:56:11 WARN]: 	at java.base/java.lang.Thread.run(Unknown Source)
[20:56:11 INFO]: [SuperiorSkyblock2-SlimeWorldModule] Unloaded island world island_00829379-d880-4a71-8345-f4d2f68b563c_normal in 19ms
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down calculation task...
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down executor
[20:56:11 INFO]: [SuperiorSkyblock2] Shutting down database executor
[20:56:12 INFO]: [SuperiorSkyblock2] Closing database. This may hang the server. Do not shut it down, or data may get lost.

If this PR is not correct, ofc I can do changes/delete it. But we can also talk about it

The patch you reference probably no longer has any effect; the new plugin is "SlimeWorldPlugin"

I'll prolly remove that patch completely.

I honestly think this is a plugin issue, not something we should band-aid, especially considering this is not an ASP-only issue. Why can't the SSB developers fix this?

@GrayR0ot
Copy link
Author

GrayR0ot commented Sep 10, 2024

This is related to how bukkit ClassLoader works now. When the plugin enter the onPhase, it will unload all classes which are not in use.
With Mongo ASP loader for example, it will unload the Mongo classes. But Until worlds save, so there is a Zip error. Meaning that Mongo classes are no longer loaded in the run-time.

To be clear, this is not possible to be fixed plugin side. This has to be handled by server jar. And Paper team hasn't planned to handle it as I heard

@AKcel222
Copy link

hmm what about an server.jar side fix from slimeworldmanager...
No possible solution ?

@AKcel222
Copy link

something as server stop command implementation from slimeworldmanager...

@CommandDan
Copy link

hmm what about an server.jar side fix from slimeworldmanager...
No possible solution ?

What do you mean with no possible solution? A few solutions have been suggested

@AKcel222
Copy link

AKcel222 commented Sep 10, 2024

then i understood it wrong.. :)

private void saveWorlds() {
        this.slimePlugin.getLoadedWorlds().forEach(world -> {
            try {
                world.getLoader().saveWorld(world.getName(), world.getPersistentDataContainer().serializeToBytes());
            } catch (IOException e) {
                throw new RuntimeException(e);
            }
            this.getServer().unloadWorld(world.getName(), true);
        });
    }
    ```
    
    This currently works for me...
    
    
    
    This would help saving worlds before the server unloads the worlds uncorrectly becuase of the issue...

@GrayR0ot
Copy link
Author

GrayR0ot commented Sep 11, 2024

image
On my side I was using this one, which is blocking the thread. But it does not works

[19:15:27 INFO]: Saving world island_1d3a9e14-03aa-4a4d-92a1-c47165b1292c_normal...
[19:15:27 WARN]: java.lang.IllegalStateException: zip file closed
[19:15:27 WARN]: 	at java.base/java.util.zip.ZipFile.ensureOpen(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.zip.ZipFile.getEntry(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.jar.JarFile.getEntry(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.jar.JarFile.getJarEntry(Unknown Source)
[19:15:27 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.findClass(PluginClassLoader.java:209)
[19:15:27 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[19:15:27 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass0(PluginClassLoader.java:169)
[19:15:27 WARN]: 	at org.bukkit.plugin.java.PluginClassLoader.loadClass(PluginClassLoader.java:164)
[19:15:27 WARN]: 	at java.base/java.lang.ClassLoader.loadClass(Unknown Source)
[19:15:27 WARN]: 	at gens.jar//libs.com.mongodb.client.gridfs.GridFSBucketImpl.uploadFromStream(GridFSBucketImpl.java:189)
[19:15:27 WARN]: 	at gens.jar//libs.com.infernalsuite.aswm.loaders.mongo.MongoLoader.saveWorld(MongoLoader.java:152)
[19:15:27 WARN]: 	at com.infernalsuite.aswm.level.SlimeLevelInstance.lambda$saveInternal$1(SlimeLevelInstance.java:160)
[19:15:27 WARN]: 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[19:15:27 WARN]: 	at java.base/java.lang.Thread.run(Unknown Source)
[19:15:27 INFO]: [SuperiorSkyblock2-SlimeWorldModule] Unloaded island world island_1d3a9e14-03aa-4a4d-92a1-c47165b1292c_normal in 20ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants