Fix ModuleType NPE if class is initialized before plugin is started

[new-sashok724]

Some of our plugins need to be loaded before AACAP, but they load its classes, including ModuleType (for example, for EnumSet). You can say we need to recode our plugins or load them after AACAP, but we can say it is bad practice to be dependent on class loading order in the first place.

If something accesses this class before plugin is started (including your own code by accident), NPE will be thrown (because AACAdditionPro.getInstance() will return null) and class initialization will fail. I fix it by lazy initializing info field.

[Janmm14]

I am just wondering what the heck your plugins are doing that they require to be loaded before AACAdditionPro.

[Photon-GitHub]

Your commit will lead to race conditions as the info is queried asynchronously which could lead to some of the more obscurely occurring NPEs. Just like Jan, I am wondering what plugins need to be loaded before AACAdditionPro.

[new-sashok724]

Our plugin needs to get events before AACAP, most importantly PlayerQuitEvent. If race conditions is concern, you can either set this field after plugin is loaded, or i can change patch to use temporary local variable for null comparison and initialization, so NPE will never happen.

[Janmm14]

Our plugin needs to get events before AACAP, most importantly PlayerQuitEvent. If race conditions is concern, you can either set this field after plugin is loaded, or i can change patch to use temporary local variable for null comparison and initialization, so NPE will never happen.

Just use the inbuilt EventHandler priority option. AACAdditionPro listens for PlayerQuitEvent on NORMAL priority (the default one). To get called before AACAdditionPro use EventPriority.LOW

[new-sashok724]

Okay, for your plugin it is possible. But for other anticheats that was not enough (it's already listening on LOWEST).

Still, i think this issue should be fixed, to avoid further problems with classloading. This NPE can be triggered just by field with value EnumSet.<ModuleType>allOf in plugin class, regardless of loading order (because field will be initialized before onEnable, on plugin class construction)

[Photon-GitHub]

It is not an issue in my opinion. There is no reason to load a plugin before AACAdditionPro and then access its API or Enums in my opinion, which would not only make quite a few changes necessary, but is against normal practice: You load a dependency of yours before the plugin, just like AAC is loaded before AACAdditionPro. If you have another use case or really require any events to be handled before AACAdditionPro that are handled with LOWEST priority by my plugin, please feel free to open an issue; I try to avoid that priority exactly because of that reason. Generally other ACs might listen with that priority, but I consider it bad practice.

[new-sashok724]

Looks like you ignored last sentence in my previous comment, please reread it.

It can be triggered even when plugin depends on AACAP, because onLoad is called first on all plugins, and only then onEnable. And even if you move instance field initialization into onLoad in AACAP, it still will be very easy to cause this NPE by initializing field with ModuleType in some plugin's class constructor (or just in place), which is obviously called both before onLoad and onEnable, but it IS legitimate API usage (because no methods are invoked on API, just API classes are loaded by field declaration/initialization).

In any case, i think it should be fixed anyway, because it is just very bad practice to be dependent on class load order, and as i said, it can lead to problems later even in AACAP itself.