cortlepp commented on PR #15843: URL: https://github.com/apache/lucene/pull/15843#issuecomment-4097407440
> I still don't think that the current code fails when native access is disabled. Have you tested this? It doesn't fail, that's not what I meant. If an exception is thrown for the 2nd/3rd catch block it will fail though and then you have the described class loading issues. My main argument is that the current implementation is brittle because it can lead to wider class loading issues for a feature that is strictly speaking optional, even on posix platforms. Besides I think that it is unwise to do the setup things we need to in `<clinit>` in general. Class loading may be triggered [by many things](https://docs.oracle.com/javase/specs/jvms/se25/html/jvms-5.html#jvms-5.5), it may even be loaded by accident on incompatible platforms e.g. windows (so when `PosixNativeAccess` should never be loaded). The new implementation fixes that, which is why I'd really prefer this or a similar pattern. > Logging the warning IF native access is disabled is actually wanted. I kind of disagree with that, but it seems to be the general stance in Lucene so I've made my peace with it. My problem with the current implementation is that I can't suppress the warning because it always attempts the native access, which triggers a JVM warning (which I can't suppress) and then a log (which I can suppress). If I want to avoid the JVM warning I would have to add a JVM flag, which is hard in my deployment scenario where I don't have direct access to the JVM, we only provide the jar. > Can you add this to the 10.x changes list in this PR already. I will take care of the backport and add modifications to the "preview" compilation in Java 21. I will open a PR for that after we merged this. 1. do you really mean 10.x, as you mentioned in it's current form this PR only works on JDK 22+ 2. As far as I understood the docs the changes.txt should only document api changes, this PR doesn't change any API (that I'm aware of) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
