search

apache / netbeans

Apache NetBeans
https://netbeans.apache.org/
Apache License 2.0
2.63k stars 841 forks source link

Handle ModuleInstall classes which are related to OSGi modules. #7624

Closed YannLeCorse closed 1 month ago

YannLeCorse commented 1 month ago

This PR is to address the issue described in the discussion #7601.

When a Netbeans modules has a ModuleInstall class, we are trying to call the validate method by reflection. If the ModuleInstall class is related to classes located in an OSGi module, we could face a NoClassDefFoundError while calling c.getDeclaredMethod("validate") because the OSGi modules have a delayed start, don't have yet been registered to the ProxyClassPackages and the systemClassLoader not being yet ready.

As the validate method is called early in the startup process, such relations should be avoid as much as possible but it should not block the startup.

Thus the proposal is to catch the exception and continue the startup process (maybe without having called the validate method).

mbien commented 1 month ago

CI appears to be stuck atm. Will try later again.

neilcsmith-net commented 1 month ago

As mentioned on the discussion, does using @OnStart instead work around your issue? I have doubts that swallowing the exception and not calling the validate method is the right approach here.

neilcsmith-net commented 1 month ago

To follow up on the discussion thread, I'm inclined to -1 this change. The issue is fairly easy to work around - see https://github.com/apache/netbeans/discussions/7601#discussioncomment-10199892

My main concern with this PR is with "continue the startup process (maybe without having called the validate method)." If we have any module that relies on OSGi classes and does validation, a refactor of the module install could start silently bypassing validation rather than throwing the exception (correctly IMO given the current limitations).

We should possibly find somewhere to better document this issue. Possibly catch and rethrow the NoClassDefFoundRethrow with explanatory text? There are limitations documented with the validation method, that also apply to class initialization time, that might be expanded on?

I would also point out the "Most modules should not need this." in the docs of ModuleInstall. There's probably a better way to achieve what's desired in the application.