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

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

Closed
wants to merge 1 commit into from

Conversation

YannLeCorse
Copy link
Contributor

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 mbien added Platform [ci] enable platform tests (platform/*) ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Jul 31, 2024
@apache apache locked and limited conversation to collaborators Jul 31, 2024
@apache apache unlocked this conversation Jul 31, 2024
@mbien
Copy link
Member

mbien commented Jul 31, 2024

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

@neilcsmith-net
Copy link
Member

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
Copy link
Member

To follow up on the discussion thread, I'm inclined to -1 this change. The issue is fairly easy to work around - see #7601 (reply in thread)

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.

@neilcsmith-net neilcsmith-net added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Jul 31, 2024
@YannLeCorse YannLeCorse deleted the osgi-startup-issue branch July 31, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) do not merge Don't merge this PR, it is not ready or just demonstration purposes. Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants