SLING-1991 : Last modified of java file should not be compared against compiled class if file is modified
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1074080 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/pom.xml b/pom.xml
index 375c01d..71c3bc3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -99,6 +99,13 @@
</dependency>
<dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.commons.classloader</artifactId>
+ <version>1.2.0</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.core</artifactId>
</dependency>
diff --git a/src/main/java/org/apache/sling/scripting/java/impl/CompilerOptions.java b/src/main/java/org/apache/sling/scripting/java/impl/CompilerOptions.java
index ab7798e..7e937f7 100644
--- a/src/main/java/org/apache/sling/scripting/java/impl/CompilerOptions.java
+++ b/src/main/java/org/apache/sling/scripting/java/impl/CompilerOptions.java
@@ -31,7 +31,7 @@
* the component configuration.
*/
public static CompilerOptions createOptions(final Dictionary<String, Object> props) {
- CompilerOptions opts = new CompilerOptions();
+ final CompilerOptions opts = new CompilerOptions();
final Boolean classDebugInfo = (Boolean)props.get(JavaScriptEngineFactory.PROPERTY_CLASSDEBUGINFO);
opts.put(Options.KEY_GENERATE_DEBUG_INFO, classDebugInfo != null ? classDebugInfo : true);
@@ -50,6 +50,18 @@
return opts;
}
+ /**
+ * Copy options
+ */
+ public static CompilerOptions copyOptions(final CompilerOptions opt) {
+ final CompilerOptions opts = new CompilerOptions();
+ opts.putAll(opt);
+
+ opts.encoding = opt.getJavaEncoding();
+
+ return opts;
+ }
+
public String getJavaEncoding() {
return this.encoding;
}
diff --git a/src/main/java/org/apache/sling/scripting/java/impl/JavaScriptEngineFactory.java b/src/main/java/org/apache/sling/scripting/java/impl/JavaScriptEngineFactory.java
index 60d4063..41a8d74 100644
--- a/src/main/java/org/apache/sling/scripting/java/impl/JavaScriptEngineFactory.java
+++ b/src/main/java/org/apache/sling/scripting/java/impl/JavaScriptEngineFactory.java
@@ -250,16 +250,20 @@
*/
public void handleEvent(Event event) {
if ( SlingConstants.TOPIC_RESOURCE_CHANGED.equals(event.getTopic()) ) {
- this.handleModification((String)event.getProperty(SlingConstants.PROPERTY_PATH));
+ this.handleModification((String)event.getProperty(SlingConstants.PROPERTY_PATH), false);
} else if ( SlingConstants.TOPIC_RESOURCE_REMOVED.equals(event.getTopic()) ) {
- this.handleModification((String)event.getProperty(SlingConstants.PROPERTY_PATH));
+ this.handleModification((String)event.getProperty(SlingConstants.PROPERTY_PATH), true);
}
}
- private void handleModification(final String scriptName) {
- final ServletWrapper wrapper = this.ioProvider.getServletCache().getWrapper(scriptName);
- if ( wrapper != null ) {
- wrapper.handleModification();
+ private void handleModification(final String scriptName, final boolean remove) {
+ if ( remove ) {
+ this.ioProvider.getServletCache().removeWrapper(scriptName);
+ } else {
+ final ServletWrapper wrapper = this.ioProvider.getServletCache().getWrapper(scriptName);
+ if ( wrapper != null ) {
+ wrapper.handleModification();
+ }
}
}
diff --git a/src/main/java/org/apache/sling/scripting/java/impl/ServletCache.java b/src/main/java/org/apache/sling/scripting/java/impl/ServletCache.java
index 02e01d5..7bed05f 100644
--- a/src/main/java/org/apache/sling/scripting/java/impl/ServletCache.java
+++ b/src/main/java/org/apache/sling/scripting/java/impl/ServletCache.java
@@ -57,7 +57,10 @@
* @param servletUri Servlet URI
*/
public void removeWrapper(String servletUri) {
- servlets.remove(servletUri);
+ final ServletWrapper wrapper = servlets.remove(servletUri);
+ if ( wrapper != null ) {
+ wrapper.destroy();
+ }
}
/**
diff --git a/src/main/java/org/apache/sling/scripting/java/impl/ServletWrapper.java b/src/main/java/org/apache/sling/scripting/java/impl/ServletWrapper.java
index 71a7a8d..17471cc 100644
--- a/src/main/java/org/apache/sling/scripting/java/impl/ServletWrapper.java
+++ b/src/main/java/org/apache/sling/scripting/java/impl/ServletWrapper.java
@@ -17,7 +17,6 @@
package org.apache.sling.scripting.java.impl;
-import java.io.IOException;
import java.util.List;
import javax.servlet.Servlet;
@@ -28,6 +27,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.sling.commons.classloader.DynamicClassLoader;
import org.apache.sling.commons.compiler.CompilationResult;
import org.apache.sling.commons.compiler.CompilerMessage;
import org.slf4j.Logger;
@@ -54,14 +54,16 @@
/** The path to the servlet. */
private final String sourcePath;
+ /** Flag for handling modifications. */
private volatile long lastModificationTest = 0L;
- private volatile Class<?> servletClass;
-
+ /** The compiled and instantiated servlet. */
private volatile Servlet theServlet;
- private long available = 0L;
+ /** Flag handling an unavailable exception. */
+ private volatile long available = 0L;
+ /** The exception thrown by the compilation. */
private volatile Exception compileException;
/**
@@ -80,8 +82,7 @@
* Call the servlet.
* @param request The current request.
* @param response The current response.
- * @throws ServletException
- * @throws IOException
+ * @throws Exception
*/
public void service(HttpServletRequest request,
HttpServletResponse response)
@@ -101,20 +102,10 @@
}
// check for compilation
- if (this.lastModificationTest == 0 ) {
+ if (this.lastModificationTest <= 0 ) {
synchronized (this) {
- if (this.lastModificationTest == 0 ) {
- try {
- // clear exception
- this.compileException = null;
- this.compile();
- } catch (Exception ex) {
- // store exception for futher access attempts
- this.compileException = ex;
- throw ex;
- } finally {
- this.lastModificationTest = System.currentTimeMillis();
- }
+ if (this.lastModificationTest <= 0 ) {
+ this.compile();
} else if (compileException != null) {
// Throw cached compilation exception
throw compileException;
@@ -125,15 +116,17 @@
throw compileException;
}
+ final Servlet servlet = getServlet();
+
// invoke the servlet
- if (theServlet instanceof SingleThreadModel) {
+ if (servlet instanceof SingleThreadModel) {
// sync on the wrapper so that the freshness
// of the page is determined right before servicing
synchronized (this) {
- theServlet.service(request, response);
+ servlet.service(request, response);
}
} else {
- theServlet.service(request, response);
+ servlet.service(request, response);
}
} catch (UnavailableException ex) {
@@ -147,7 +140,6 @@
(HttpServletResponse.SC_SERVICE_UNAVAILABLE,
ex.getMessage());
logger.error("Java servlet {} is unavailable.", this.sourcePath);
- return;
}
}
@@ -161,32 +153,80 @@
}
}
- /** Handle the modification. */
+ /**
+ * Handle the modification.
+ */
public void handleModification() {
- this.lastModificationTest = 0;
+ logger.debug("Received modification event for {}", this.sourcePath);
+ this.lastModificationTest = -1;
}
- private void compile() throws Exception {
- final CompilationUnit unit = new CompilationUnit(this.sourcePath, className, ioProvider);
- final CompilationResult result = this.ioProvider.getCompiler().compile(new org.apache.sling.commons.compiler.CompilationUnit[] {unit},
- ioProvider.getOptions());
-
- final List<CompilerMessage> errors = result.getErrors();
- if ( errors != null && errors.size() > 0 ) {
- throw CompilerException.create(errors, this.sourcePath);
+ /**
+ * Check if the used classloader is still valid
+ */
+ private boolean checkReload() {
+ final Servlet servlet = this.theServlet;
+ final Class<?> servletClass = servlet != null ? servlet.getClass() : null;
+ if ( servletClass != null && servletClass.getClassLoader() instanceof DynamicClassLoader ) {
+ return !((DynamicClassLoader)servletClass.getClassLoader()).isLive();
}
- if ( result.didCompile() || this.theServlet == null ) {
- destroy();
+ return false;
+ }
- this.servletClass = result.loadCompiledClass(this.className);
- final Servlet servlet = (Servlet) servletClass.newInstance();
- servlet.init(config);
+ /**
+ * Get the servlet class - if the used classloader is not valid anymore
+ * the class is reloaded.
+ */
+ public Servlet getServlet()
+ throws Exception {
+ // check if the used class loader is still alive
+ if (this.checkReload()) {
+ synchronized (this) {
+ if (this.checkReload()) {
+ this.compile();
+ }
+ }
+ }
+ return theServlet;
+ }
- theServlet = servlet;
+ /**
+ * Compile the servlet java class
+ * and instantiate the servlet
+ */
+ private void compile()
+ throws Exception {
+ logger.debug("Compiling {}", this.sourcePath);
+ // clear exception
+ this.compileException = null;
+ try {
+ final CompilerOptions opts = (this.lastModificationTest == -1 ? this.ioProvider.getForceCompileOptions() : this.ioProvider.getOptions());
+ final CompilationUnit unit = new CompilationUnit(this.sourcePath, className, ioProvider);
+ final CompilationResult result = this.ioProvider.getCompiler().compile(new org.apache.sling.commons.compiler.CompilationUnit[] {unit},
+ opts);
+ final List<CompilerMessage> errors = result.getErrors();
+ if ( errors != null && errors.size() > 0 ) {
+ throw CompilerException.create(errors, this.sourcePath);
+ }
+ if ( result.didCompile() || this.theServlet == null ) {
+ destroy();
+ final Class<?> servletClass = result.loadCompiledClass(this.className);
+ final Servlet servlet = (Servlet) servletClass.newInstance();
+ servlet.init(this.config);
+
+ this.theServlet = servlet;
+ }
+ } catch (final Exception ex) {
+ // store exception for futher access attempts
+ this.compileException = ex;
+ throw ex;
+ } finally {
+ this.lastModificationTest = System.currentTimeMillis();
}
}
+ /** Compiler exception .*/
protected final static class CompilerException extends ServletException {
private static final long serialVersionUID = 7353686069328527452L;
diff --git a/src/main/java/org/apache/sling/scripting/java/impl/SlingIOProvider.java b/src/main/java/org/apache/sling/scripting/java/impl/SlingIOProvider.java
index fdba2e7..a7ef07a 100644
--- a/src/main/java/org/apache/sling/scripting/java/impl/SlingIOProvider.java
+++ b/src/main/java/org/apache/sling/scripting/java/impl/SlingIOProvider.java
@@ -30,6 +30,7 @@
import org.apache.sling.api.resource.ResourceMetadata;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.commons.compiler.JavaCompiler;
+import org.apache.sling.commons.compiler.Options;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,8 +46,12 @@
private final JavaCompiler compiler;
+ /** Options for compilation. */
private final CompilerOptions options;
+ /** Options for compilation including the force compile option. */
+ private final CompilerOptions forceCompileOptions;
+
/**
* Servlet cache.
*/
@@ -60,6 +65,8 @@
this.requestResourceResolver = new ThreadLocal<ResourceResolver>();
this.compiler = compiler;
this.options = options;
+ this.forceCompileOptions = CompilerOptions.copyOptions(options);
+ this.forceCompileOptions.put(Options.KEY_FORCE_COMPILATION, true);
}
void destroy() {
@@ -84,6 +91,10 @@
return this.options;
}
+ public CompilerOptions getForceCompileOptions() {
+ return this.forceCompileOptions;
+ }
+
public ServletCache getServletCache() {
return this.servletCache;
}
