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

Application crash while looking for module in JDBC library #165

Closed
ghost opened this issue Dec 9, 2016 · 17 comments
Closed

Application crash while looking for module in JDBC library #165

ghost opened this issue Dec 9, 2016 · 17 comments
Labels
cannot reproduce Submitter should provide a reproducible scenario

Comments

@ghost
Copy link

ghost commented Dec 9, 2016

Description

When Discord4J tries to load JDBC class (in my case - Xerial's JDBC), Class.forName() causes triggered actions inside this class BEFORE declaration of another required classes (? not sure what's actually going on inside JDBC class), so we have exception like this: http://pastebin.com/Dnr7a3p9
In fact, this can happen with ANY another class in another libraries that has static { } blocks under certain circumstances. So, basicly, the simplest and the best solution avaible is to remove static module loading from Discord4J, Class.forName() is really bad. This occurs at sx.blah.discord.modules.ModuleLoader.loadExternalModules(ModuleLoader.java:223), as you can see from exception higher.

Steps to Reproduce

  1. Create Discord4J module
  2. Add class that uses static {} blocks in way it uses libraries like xerial's JDBC (or add library like it)
  3. Profit, you're crashed forever!

...

Expected behavior: module successfully loaded and everything works well

Actual behavior: Class.forName() used to load class breaks everything

Stacktrace (if applicable): http://pastebin.com/Dnr7a3p9

Version affected: 2.6.1

P.S. Sorry for my english if it is bad :(

@ghost
Copy link
Author

ghost commented Dec 9, 2016

Referenced this in Xerial's JDBC library repo as well

@ArsenArsen
Copy link
Contributor

I think this is only supposed to skip to the next module, but I don't see how this could be because of D4J. I'll test it.

@ghost
Copy link
Author

ghost commented Dec 10, 2016

As i said, when Class#forName called for one of classes inside module jar, it executes static blocks. Actually this is fault of both D4J AND JDBC. And as I referenced, i have created an issue on Xerial's repo, but I think Discord4J SHOULD predict such kind of problems due to using static blocks in some well-known libraries. Exception occurs in module, but caused by the base application.

@darichey darichey added the cannot reproduce Submitter should provide a reproducible scenario label Dec 11, 2016
@darichey
Copy link
Member

Created a module with the linked sql library shaded into the jar and it loaded just fine. Not sure what your issue is.

@ghost
Copy link
Author

ghost commented Dec 12, 2016

I'll try it again today.

@ArsenArsen
Copy link
Contributor

Ok, I have been watching over this thread and I've gotta say how many wrong things I have found.

  1. JDBC is not a library, its a java internal binding system for relational database drivers.
  2. Xerial's SQLite library works flawlessly on all OS' I use so I do not think its them.
  3. You probably just failed to shade everything in. Post your JAR and/or sources.

I agree on, the part where Discord4J should ignore the modules that failed to load, though.

@ghost
Copy link
Author

ghost commented Dec 12, 2016

Tried by myself
https://github.com/TaylerNest/d4j-sqlite-temp
Still not working.

@darichey
Copy link
Member

I feel like I must be missing something. A ClassNotFoundException is thrown here because there is no such class as org.xerial.SQLite. If you change it to a class that actually exists in sqlite-jdbc, it works just fine.

@ArsenArsen
Copy link
Contributor

ArsenArsen commented Dec 13, 2016

This one is not D4J's fault. As Panda stated your'e initializing the wrong class. Plus there is no need to do that, JDBC does it for you. Remove that line and try-catch.

I'll make a pull request to make the module get ignored and continue loading as per usual.

@ArsenArsen
Copy link
Contributor

I am actually able to reproduce this @GrandPanda ..

It is on a linux machine loading the jar that he has. Looks to me personally like a Java hiccup, I've hacked together a workaround for it. Will PR now.

@ghost
Copy link
Author

ghost commented Dec 13, 2016

Same thing on my Windows 10
Also, fixed error in my repo, thanks - i've been busy yesterday and made this mistake. But this doesn't solves the problem - enable method is never executed, crash happens BEFORE enabling modules - as i said, on classes scanning Class.forName() is executed for each class in jar, but it triggers static { } blocks execution.
But how to fix it - i don't actually even imagine, it seems unsolvable due to static module loading. No offence.

@ghost
Copy link
Author

ghost commented Dec 13, 2016

If this isn't goes to be really fixed, it will be better to notify module developers that they may need to link dependencies using alternative ways (maybe dynamic loading) and put them into subfolder or inside jar as is if they want to really avoid such kind of errors.

Also, have you ever tried to use debugger?

@ArsenArsen
Copy link
Contributor

It can be temporarly resolved, but there is absolutely zero need for such thing. Try to update sqlite and remove the static block. @GrandPanda did it successfully after all, meaning its a problem on your end(s?).

@ghost
Copy link
Author

ghost commented Dec 13, 2016

But as I know, it seems like @GrandPanda used github repo - is it official now? How it differs with bitbucket repo, are they just moved to github?

@ArsenArsen
Copy link
Contributor

@TaylerNest Does not matter, just shade it in.

@darichey
Copy link
Member

darichey commented Dec 14, 2016

Yeah, just tried it again to make sure I'm not crazy. It works perfectly fine. I've attached the built module jar which produces this output: http://hastebin.com/agazebehus.md. There's a notice on the bitbucket repo that the project has been moved to github.

moduletest.zip

@ghost
Copy link
Author

ghost commented Dec 15, 2016

Now using the newest version of Xerial's SQLite library, and it works. Sorry for my stupidity.

@ghost ghost closed this as completed Dec 15, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cannot reproduce Submitter should provide a reproducible scenario
Projects
None yet
Development

No branches or pull requests

2 participants