[RFC] Netx: some native libraries are not found

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2010-06-29 16:35]:
> On 06/29/2010 02:06 PM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2010-06-21 10:33]:
> >>Hi,
> >>
> >>There is a bug in the current version of Netx that fails to find
> >>some native libraries [1][2].
> >>
> >>In the current implementation of Netx, one classloader is created
> >>per JNLP file; for JNLP files loaded as extensions, the classloaders
> >>share information about where to find resources. Unfortunately, they
> >>do not share any information about where to find native libraries.
> >>This makes Web Start applications with extensions that use native
> >>libraries fail. The proposed patch [3] fixes this.
> >>
> >>Any comments concerns? Should I go ahead and commit this fix?
> >>
> >
> >Patch looks good to me. Assuming you have tested it, please go ahead and
> >commit.
> >
> 
> On furthur testing, I found an issue with the patch I had posted. I
> had to change the check for extLoader and baseLoader from
>  if (baseLoader != null)
> to
>  if (baseLoader != null && baseLoader != loader)
> which makes sense to me: adding urls and native directories
> repeatedly to the same loader has no point.
>

Why would the directories and urls get added again? That would only
happen if the extension loaders had duplicate paths, which is really a
bug in the jnlp...

Or do you know of cases where it does it multiple times even if the jnlp
specifies it only once?

Cheers,
Deepak

> Ok to commit?
> 
> Cheers,
> Omair

> tree bba041892d23
> parent e7c3958eee03
> author Omair Majid <omajid at redhat.com> 1277840890 14400
> committer Omair Majid <omajid at redhat.com> 1277840890 14400
> revision 2141
> branch default
> 
> Netx: allow jnlp classloaders to share native libraries
> 2010-06-29  Omair Majid <omajid at redhat.com>
> 
>     * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>     nativeDirectories: New variable. Contains a list of directories that
>     contain native libraries.
>     (getInstance): Tell other classloaders about the native directory.
>     (getNativeDir): Add the new native directory to nativeDirectories.
>     (addNativeDirectory): New function.
>     (getNativeDirectories): New function.
>     (findLibrary): Look in all the native directories for the native library.
> diff --git a/ChangeLog b/ChangeLog
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,14 @@
> +2010-06-29  Omair Majid <omajid at redhat.com>
> +
> +	* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> +	nativeDirectories: New variable. Contains a list of directories that
> +	contain native libraries.
> +	(getInstance): Tell other classloaders about the native directory.
> +	(getNativeDir): Add the new native directory to nativeDirectories.
> +	(addNativeDirectory): New function.
> +	(getNativeDirectories): New function.
> +	(findLibrary): Look in all the native directories for the native library.
> +
>  2010-06-29  Omair Majid <omajid at redhat.com>
>  
>  	* netx/net/sourceforge/jnlp/services/XSingleInstanceService.java
> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> @@ -32,6 +32,7 @@
>  import java.security.Permissions;
>  import java.security.PrivilegedAction;
>  import java.util.ArrayList;
> +import java.util.Collections;
>  import java.util.Enumeration;
>  import java.util.HashMap;
>  import java.util.LinkedList;
> @@ -83,6 +84,9 @@
>      /** the directory for native code */
>      private File nativeDir = null; // if set, some native code exists
>  
> +    /** a list of directories that contain native libraries */
> +    private List<File> nativeDirectories = Collections.synchronizedList(new LinkedList<File>());
> +
>      /** security context */
>      private AccessControlContext acc = AccessController.getContext();
>  
> @@ -237,18 +241,22 @@
>  		        // loader for this unique key. Check.
>  		        JNLPClassLoader extLoader = (JNLPClassLoader) urlToLoader.get(uniqueKey);
>  
> -		        if (extLoader != null) {
> +		        if (extLoader != null && extLoader != loader) {
>  		            for (URL u : loader.getURLs())
>  		                extLoader.addURL(u);
> +		            for (File nativeDirectory: loader.getNativeDirectories())
> +		                extLoader.addNativeDirectory(nativeDirectory);
>  
>  		            loader = extLoader;
>  		        }
>  
>                  // loader is now current + ext. But we also need to think of 
>                  // the baseLoader
> -		        if (baseLoader != null) {
> +		        if (baseLoader != null && baseLoader != loader) {
>                      for (URL u : loader.getURLs())
>                          baseLoader.addURL(u);
> +                    for (File nativeDirectory: loader.getNativeDirectories())
> +                        baseLoader.addNativeDirectory(nativeDirectory);
>  
>                      loader = baseLoader;
>                  } 
> @@ -716,29 +724,47 @@
>  
>          if (!nativeDir.mkdirs()) 
>              return null;
> -        else
> +        else {
> +            // add this new native directory to the search path
> +            addNativeDirectory(nativeDir);
>              return nativeDir;
> +        }
> +    }
> +
> +    /**
> +     * Adds the {@link File} to the search path of this {@link JNLPClassLoader}
> +     * when trying to find a native library
> +     */
> +    protected void addNativeDirectory(File nativeDirectory) {
> +        nativeDirectories.add(nativeDirectory);
> +    }
> +
> +    /**
> +     * Returns a list of all directories in the search path of the current classloader
> +     * when it tires to find a native library.
> +     * @return a list of directories in the search path for native libraries
> +     */
> +    protected List<File> getNativeDirectories() {
> +        return nativeDirectories;
>      }
>  
>      /**
>       * Return the absolute path to the native library.
>       */
>      protected String findLibrary(String lib) {
> -        if (nativeDir == null)
> -            return null;
> -
>          String syslib = System.mapLibraryName(lib);
>  
> -        File target = new File(nativeDir, syslib);
> -        if (target.exists())
> -            return target.toString();
> -        else {
> -            String result = super.findLibrary(lib);
> -            if (result != null)
> -                return result;
> +        for (File dir: getNativeDirectories()) {
> +            File target = new File(dir, syslib);
> +            if (target.exists())
> +                return target.toString();
> +        }
>  
> -            return findLibraryExt(lib);
> -        }
> +        String result = super.findLibrary(lib);
> +        if (result != null)
> +            return result;
> +
> +        return findLibraryExt(lib);
>      }
>  
>      /**