search

eclipse-aspectj / aspectj

Other
272 stars 82 forks source link

set cause on failed class load #265

Closed euclio closed 6 months ago

euclio commented 6 months ago

If classloading fails due to an IOException, this patch sets the cause of the thrown ClassNotFoundException so that the root cause can be found.

This issue was discovered after my machine was throwing "too many open files" when attempting to open a JAR, but the class loader was hiding this cause.

Fixes #266.

kriegaex commented 6 months ago

The change looks reasonable, thank you. An integration test would be helpful. Can you tell me how to reproduce the problem? I could find out by myself, but do not have much time. Please provide a minimal reproducer. I can think about how to convert it into an IT myself, if our test structure is too confusing for you.

euclio commented 6 months ago

It's probably difficult to reproduce in a unit test environment. The issue happens when the class being loaded exists, but the loading fails with an IOException. In my case, it was because it was trying to open the JAR to load the class but I had hit the open file limit for my machine.

kriegaex commented 6 months ago

I said integration test, not unit test. In a unit test, probably it would be easier to reproduce by stubbing a mock method. But thanks for the pointer in the right direction, I will think about it. Maybe a unit test is actually sufficient, maybe I come up with an idea for an IT.

kriegaex commented 6 months ago

@euclio, do you have one of your stack traces (after the change) for me? That would help to retrace the execution path below ExtensibleURLClassLoader::findClass.

I found out how to reproduce it in a unit test myself, but the stack trace would still be nice. Maybe you can edit the PR description and add it there for documentary purposes.