ViktorC / PP4J

A multiprocessing library for Java that features process pool implementations and a flexible API.
Apache License 2.0
32 stars 6 forks source link

Can we use PP4J inside a web application deployed on tomcat? #2

Closed pvb05 closed 6 years ago

pvb05 commented 6 years ago

Hi,

I am trying to use PP4J as part of a web application deployed on tomcat. I am initializing the JVM pool as part of Spring bean initialization. I see that it gets stuck on the initialization and doesnt allow the application to be deployed. Do you know why that happens?

Can you throw some light on that?

thanks, Pradeep B.

ViktorC commented 6 years ago

Hi Pradeep,

I believe the JVM processes cannot be launched.

The command that is used to launch the processes looks something like this ${JAVA_HOME}/bin/java [other options] -cd ${CLASSPATH} net.viktorc.pp4j.impl.JavaProcess. The other options are optional and depend on the JavaProcessOptions instance you use while the Java home folder and the class path are resolved using System.getProperty("java.home") and System.getProperty("java.class.path") respectively. See line 272-304 in JavaProcessPoolExecutor.

My guess would be that relying on System.getProperty("java.class.path") does not work in web containers as they use dynamic class paths. Thus I conclude that the Java process pool is not compatible with Tomcat at the moment, but I will try to find a solution that works in containers as well and keep this thread up to date.

Best regards, Viktor

pvb05 commented 6 years ago

Hi Viktor, i had some workarounds but thought of looking at the code you mentioned and added the below logic to use instead of just using the System.getProperty("java.class.path");

String javaClassPath = System.getProperty("java.class.path"); StringBuffer classpathBuffer = new StringBuffer(javaClassPath); URLClassLoader loader = (URLClassLoader) this.getClass().getClassLoader(); URL[] urls = loader.getURLs(); for(URL url : urls) { classpathBuffer.append(File.pathSeparator).append(url.getFile()); } String classPath = classpathBuffer.toString();

Does this change look ok?

thanks, Pradeep V.B.

ViktorC commented 6 years ago

Hi Pradeep,

Thank you for the suggestion!

I am not sure, but I think duplicated class path entries might cause some problems (e.g. if the pool is not run in a container). It would probably be safer if you split the string returned by System.getProperty("java.class.path") by File.pathSeparator, load the sub-strings into a string set, insert the URLs of the class loader into this set, and build the class path out of the entries of the set. It might also be a good idea to check if the class loader is really an instance of URLClassLoader.

With these changes, I would gladly merge your pull request. In case you are not interested in contributing, I can also add the changes myself and release a new version.

As for a long term solution, this could only be one if Tomcat and other containers used the same class loader to load all application and dependency classes. Do you know if they do?

Cheers, Viktor

pvb05 commented 6 years ago

If you can get me the fix, it would help me out. As far as i think, using approach, the pool uses the classpath entries of the web application from where the pool was created. Do you think there will be problems if there are multiple pools with different classpath entries started from different web applications within tomcat?

ViktorC commented 6 years ago

Hi Pradeep,

I have implemented the fix and released a new version (2.1) which should also be available from the Maven Central Repository within about an hour.

This is what the fix looks like:

String classPath = System.getProperty("java.class.path");
ClassLoader classLoader = this.getClass().getClassLoader();
if (classLoader instanceof URLClassLoader) {
    @SuppressWarnings("resource")
    URLClassLoader urlClassLoader = (URLClassLoader) classLoader;
    Set<String> classPathEntries = new HashSet<>(Arrays.asList(classPath.split(File.pathSeparator)));
    for (URL url : urlClassLoader.getURLs()) {
        try {
            classPathEntries.add(Paths.get(url.toURI()).toAbsolutePath().toString());
        } catch (URISyntaxException e) {
            continue;
        }
    }
    classPath = String.join(File.pathSeparator, classPathEntries);
}

Multiple process pools started from different web applications should be fine. I was just not sure if Tomcat uses a single class loader per application. However, based on my tests, it seems it does and therefore your proposed approach works fine.

Thank you and best regards, Viktor

pvb05 commented 6 years ago

Thanks Viktor, I will take the new release and test it. I will let you know if i find any issue.

Thanks again for taking care of this.

cheers, Pradeep V.B.