Guard against NoClassDefFoundError when trying to load Unsafe. #7432
Conversation
Motivation: OSGI and other enviroments may not allow to even load Unsafe which will lead to an NoClassDefFoundError when trying to access it. We should guard against this. Modifications: Catch NoClassDefFoundError when trying to load Unsafe. Result: Be able to use netty with a strict OSGI config.
normanmaurer
force-pushed
the
guard_against_no_class_def_found
branch
from
7230f7b
to
466a97f
Compare
| @@ -96,6 +96,10 @@ public Object run() { | |||
| return e; | |||
| } catch (IllegalAccessException e) { | |||
| return e; | |||
| } catch (NoClassDefFoundError e) { | |||
There was a problem hiding this comment.
I think LinkageError might be a little more robust here, would love to hear what @jasontedor has to say.
There was a problem hiding this comment.
Yeah maybe that would be a good idea!
There was a problem hiding this comment.
Thanks for the ping @johnou. I think the most specific error possible should be caught. If this throws a NoClassDefFoundError in an OSGI situation, that I think that's what should be caught here only.
There was a problem hiding this comment.
Also, before merging, I would recommend thinking about my comment on the original issue. It's hard to support something that you do not have experience with, tests for, etc. so maybe the best path is not to make a code change here at all (because a future refactoring might "break" it again). Instead, there's an out here for users in OSGI already: either add the dependency needed, or set a flag to skip trying to use unsafe.
There was a problem hiding this comment.
There is actually a Netty OSGI test suite 💯
There was a problem hiding this comment.
yep ... I will try to add a test.. not idea if I will be successful tho =P
There was a problem hiding this comment.
@normanmaurer not as easy as trying to load the class with the PaxExam runner is it? 🧀
There was a problem hiding this comment.
Apparently it does not do much of anything as it's not catching an issue for OSGI environments that do not add the necessary dependency for unsafe that has been there for over a year (#5640). And I question having that with two of the main maintainers saying that they do not have OSGI knowledge to maintain support for it. And none of this negates my point that there's an out for OSGI already without a change needed in Netty.
There was a problem hiding this comment.
@jasontedor I would still argue that this specific exception is not 100% related to OSGI and that Netty should be able to detect if unsafe is available or not and if it is not, fallback.
There was a problem hiding this comment.
Okay, I agree with you.
|
Unfortunately I was not able to write a test with paxexam (maybe because I don't know enough about it). That said I have read multiple sources in the internet that state that the |
Motiviation: The OSGi Test suite runs without access to sun.misc.Unsafe, and so is a good place to put a test to avoid regressing netty#6548. Modification: Added a test-case that failed before netty#7432. Result: Test for fix included.
Motiviation: The OSGi Test suite runs without access to sun.misc.Unsafe, and so is a good place to put a test to avoid regressing netty#6548. Modification: Added a test-case that failed before netty#7432. Result: Test for fix included.
Motivation:
OSGI and other enviroments may not allow to even load Unsafe which will lead to an NoClassDefFoundError when trying to access it. We should guard against this.
Modifications:
Catch NoClassDefFoundError when trying to load Unsafe.
Result:
Be able to use netty with a strict OSGI config.