Improve GlobalScreen

[xeruf]

• Set the default loglevel to WARNING - fixes Getting mouse debug inside eclipse console #238
• Move library initialization out of static block to registerNativeHook - fixes Do not load statically #249
• Improve javadoc

I was not yet able to test the integrity of these changes due to #250.

[sghpjuikit]

The boolean flag may need to be volatile.

[xeruf]

Actually, if that is necessary, then it has to be atomic

[kwhat]

so my big concern with this is will it handle the standard Java LogManager properties? See https://docs.oracle.com/cd/E17277_02/html/GettingStartedGuide/managelogging.html#logginglevels

Most of the people complaining about this have no idea how Java's logging facility works.

[xeruf]

Because it isn't good. I have already mentioned slf4j, but it is your choice whether you want to use it. Otherwise, there should be a way of checking these properties, but as long as #250 is unresolved I can hardly test it.

[kwhat]

Why do we need a boolean flag? You know for #249, you can override the library loading by using the NativeLibraryLocator inteface. See: https://github.com/kwhat/jnativehook/blob/2.1/src/java/org/jnativehook/NativeLibraryLocator.java

I am still not sure what issue you are trying to solve with this... If the native library is not loaded, statically or via java.library.path property, you will still get undefined method exceptions because the methods declared with the native keyword will not be found.

[xeruf]

The problem was that we had a bug where, as soon as the JNativeHook JNA libraries were loaded on Linux, the JavaFX FileChooser crashed. Don't ask me how this worked, but that's how it was. This problem is gone now, but if there is a possibility of breaking something, it should only execute once you actually register, not when the class is accessed in any way. That the native functions aren't defined until then shouldn't be the problem, since registering is afaik the first thing you have to do anyways.
I am not very keen on JNA, but this seemed to me like a proper solution to this.

The boolean flag is for not loading the library twice if it is registered twice. Btw @sghpjuikit it doesn't have to be volatile, since it is only ever accessed from one thread.

[kwhat]

but if there is a possibility of breaking something, it should only execute once you actually register, not when the class is accessed in any way.

You missed the point, as soon as the ClassLoader loads the GlobalScreen these native methods must be defined. You cannot wait until the class is loaded and a method is executed. You really should be using the java.library.path or use your own network based NativeLibraryLocator. I will repeat, you CANNOT load the class with undefined native methods. you will get a java.lang.UnsatisfiedLinkError: Native method not found: exception right after the static {} context is initialized.

Because it isn't good. I have already mentioned slf4j

Why isn't it good? slf4j isn't a bad idea, but it does introduce another run-time dependency. Log4j is however out of the question.

I am not very keen on JNA

This library is written in JNI, not JNA. Yes, JNI/JNA is really the only way to make this work.

[xeruf]

Okay, I will revert the initialisation move.

You do know that slf4j is only the adapter, with log4j being one binding to it? In a library you should always take slf4j over log4j.

[sghpjuikit]

@kwhat I work with @Xerus2000 on the issue. Our 'problem' was that we needed to avoid loading GlobalScreen.class because it would immediately load some native library that in turn caused problematic behavior on the user system. Hence we tried to isolate the entire library by hiding it behind a facade that loaded lazily on demand, which we had under control (i.e., could refuse to serve on affected platforms).

Personally, I had no idea the native library had to be loaded in static initialization block (I believe you that it must). So we thought that it would be a good idea to try to put the on-demand loading into this very library - now it seems to be an impossible endeavor. And I do not see how java.library.path or NativeLibraryLocator can really help avoid loading the native resources.

So I am correct in assuming that in situation like this, the only way to prevent the native library from being loaded is to not trigger initialization of the class and subsequent invocation of the static block? We can live with that, it just seems a little fragile.

Or perhaps, would overriding NativeLibraryLocator.getLibraries to return empty list cause no native resources to be loaded and do so with no exception raised? It seems to me the library assumes from its presence that it will be used and that loading native resources in some way is required.

[kwhat]

Hence we tried to isolate the entire library by hiding it behind a facade that loaded lazily on demand, which we had under control (i.e., could refuse to serve on affected platforms).

So there is a bug in Windows that prevents the deletion of the native library on unload. I bring this up because I believe that the solutions to both these problems intersect on a custom class loader for the GlobalScreen. I have no idea how this should/can be implemented, but in theory the code in the class loader will control how GlobalScreen is loaded and you maybe able to figure out a way to work around the issue you found by moving all this static library loading to this class loader. Take a look at https://web.archive.org/web/20140704120535/http://www.codethesis.com/blog/unload-java-jni-dll and https://community.oracle.com/thread/1550086 for some context. If you need help with this approach, I can probably assist.

Or perhaps, would overriding NativeLibraryLocator.getLibraries to return empty list cause no native resources to be loaded and do so with no exception raised?

This approach may work, however, I think you will still run into the java.lang.UnsatisfiedLinkError as soon as GlobalScreen gets loaded by the default class loader. You may try to implement the native methods with a no-op or something to that effect to prevent this error.

[kwhat]

You do know that slf4j is only the adapter, with log4j being one binding to it? In a library you should always take slf4j over log4j.

Yes, I believe that slf4j requires the following at runtime:

<dependency>
    <groupId>org.slf4j</groupId>
    <artifactId>slf4j-api</artifactId>
    <version>1.7.25</version>
</dependency>

If you have a compelling case for slf4j, it can be added for 3.0.

[sghpjuikit]

No need for assistance, we have actually written custom class loaders in our project. I'll think about the idea. I think we will end up using the facade that allows us to lazily load the class without class loader magic. Thx for the input.

[xeruf]

Alright, I have removed the initialization changes. What about merging it now?

[xeruf]

@kwhat any news?

[xeruf]

@kwhat you still alive? ^^

[kwhat]

Unfortunately, yes.

[xeruf]

@kwhat can you merge this? I mean, it's only a few small improvements?

[goxr3plus]

Merge and let's have a dinner.

[deinhofer]

Hence we tried to isolate the entire library by hiding it behind a facade that loaded lazily on demand, which we had under control (i.e., could refuse to serve on affected platforms).

So there is a bug in Windows that prevents the deletion of the native library on unload. I bring this up because I believe that the solutions to both these problems intersect on a custom class loader for the GlobalScreen. I have no idea how this should/can be implemented, but in theory the code in the class loader will control how GlobalScreen is loaded and you maybe able to figure out a way to work around the issue you found by moving all this static library loading to this class loader. Take a look at https://web.archive.org/web/20140704120535/http://www.codethesis.com/blog/unload-java-jni-dll and https://community.oracle.com/thread/1550086 for some context. If you need help with this approach, I can probably assist.

Or perhaps, would overriding NativeLibraryLocator.getLibraries to return empty list cause no native resources to be loaded and do so with no exception raised?

This approach may work, however, I think you will still run into the java.lang.UnsatisfiedLinkError as soon as GlobalScreen gets loaded by the default class loader. You may try to implement the native methods with a no-op or something to that effect to prevent this error.

I had an issue where it could happen that GloabalScreen.registerNativeHook() could have been called several times which than led to an error.
So I did a manual cleanup of the the extracted .dlls in the temp directory first:

        File tempDir = new File(System.getProperty("java.io.tmpdir"));
        File[] tempDLLs = tempDir.listFiles(new FilenameFilter() {

            @Override
            public boolean accept(File dir, String name) {
                if (name.toLowerCase().startsWith("jnativehook") && name.endsWith(".dll")) {
                    return true;
                }

                return false;
            }
        });
        for (File tempDLL : tempDLLs) {
            tempDLL.delete();
        }