Skip to content

Commit

Permalink
PathTimestamp list replaces the paths and lastModifiedDates arrays
Browse files Browse the repository at this point in the history 
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
  • Loading branch information
@dmatej
dmatej committed Jan 15, 2023
1 parent f7f8797 commit 03bbaa5
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 89 deletions.
21 changes: 12 additions & 9 deletions appserver/web/war-util/src/main/java/org/glassfish/web/loader/JarFileManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class JarFileManager implements Closeable {
private static final int SECONDS_TO_CLOSE_UNUSED_JARS = Integer
.getInteger("org.glassfish.web.loader.unusedJars.secondsToClose", 60);
private static final int SECONDS_TO_CHECK_UNUSED_JARS = Integer
.getInteger("org.glassfish.web.loader.unusedJars.secondsToRunCheck", 10);
.getInteger("org.glassfish.web.loader.unusedJars.secondsToRunCheck", 15);

private static final Logger LOG = System.getLogger(JarFileManager.class.getName());

Expand Down Expand Up @@ -142,14 +142,20 @@ ResourceEntry findResource(String name, String path, File loaderDir, boolean ant
LOG.log(DEBUG, "Failed to create URL from " + entry.codeBase + " and path " + path, e);
return null;
}
entry.lastModified = file.lastModified();
final int contentLength = (int) jarEntry.getSize();
final InputStream binaryStream;
try {
entry.manifest = jarFile.getManifest();
binaryStream = jarFile.getInputStream(jarEntry);
} catch (IOException e) {
LOG.log(DEBUG, "Failed to get manifest or input stream for " + jarFile.getName(), e);
LOG.log(DEBUG, "Failed to get manifest for " + jarFile.getName(), e);
return null;
}
entry.lastModified = file.lastModified();
final int contentLength = (int) jarEntry.getSize();
try (InputStream binaryStream = jarFile.getInputStream(jarEntry)) {
if (binaryStream != null) {
entry.readEntryData(name, binaryStream, contentLength, jarEntry);
}
} catch (IOException e) {
LOG.log(DEBUG, "Failed to read entry data for " + name, e);
return null;
}

Expand All @@ -160,9 +166,6 @@ ResourceEntry findResource(String name, String path, File loaderDir, boolean ant
extractResources(loaderDir, path);
}
}
if (binaryStream != null) {
entry.readEntryData(name, binaryStream, contentLength, jarEntry);
}
return entry;
}
} finally {
Expand Down
2 changes: 1 addition & 1 deletion appserver/web/war-util/src/main/java/org/glassfish/web/loader/ResourceEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class ResourceEntry {


/**
* Reads the resource's binary data from the given input stream.
* Reads the resource's binary data from the given input stream and closes the stream.
*/
void readEntryData(String name, InputStream binaryStream, int contentLength, JarEntry jarEntry) {
byte[] bytes = new byte[contentLength];
Expand Down
126 changes: 47 additions & 79 deletions appserver/web/war-util/src/main/java/org/glassfish/web/loader/WebappClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import java.text.MessageFormat;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
Expand Down Expand Up @@ -216,23 +215,17 @@ public final class WebappClassLoader extends GlassfishUrlClassLoader
*/
private final JarFileManager jarFiles = new JarFileManager();

/**
* The list of JARs in {@link #WEB_INF_LIB}, in the order they should be searched
* for locally loaded classes or resources. This list serves to check if files changed.
*/
private List<String> jarNames = new ArrayList<>();

/**
* The list of JARs last modified dates, in the order they should be
* searched for locally loaded classes or resources.
*/
private long[] lastModifiedDates = new long[0];
private final ConcurrentLinkedQueue<PathTimestamp> pathTimestamps = new ConcurrentLinkedQueue<>();

/**
* The list of resources which should be checked when checking for
* modifications.
* The list of JARs in {@link #WEB_INF_LIB}, in the order they should be searched
* for locally loaded classes or resources. This list serves to check if files changed.
*/
private String[] paths = new String[0];
private List<String> jarNames = new ArrayList<>();

/**
* A list of read File and Jndi Permission's required if this loader
Expand Down Expand Up @@ -537,15 +530,8 @@ public void addJar(String filePath, File file) {
}

try {
// Register the JAR for tracking
final long lastModified = getResourceAttributes(filePath).getLastModified();
final String[] newPaths = Arrays.copyOf(paths, paths.length + 1);
newPaths[paths.length] = filePath;
paths = newPaths;

final long[] newLastModified = Arrays.copyOf(lastModifiedDates, lastModifiedDates.length + 1);
newLastModified[lastModifiedDates.length] = lastModified;
lastModifiedDates = newLastModified;
pathTimestamps.add(new PathTimestamp(filePath, lastModified));
} catch (NamingException e) {
LOG.log(DEBUG, "Could not get resource attributes from JNDI for " + filePath, e);
}
Expand Down Expand Up @@ -587,33 +573,26 @@ public void start() {
public boolean modified() {
checkStatus(LifeCycleStatus.RUNNING);
// Checking for modified loaded resources
final int pathsLength = paths.length;
// A rare race condition can occur in the updates of the two arrays
// It's totally ok if the latest class added is not checked (it will
// be checked the next time
final int lastModifiedDatesLength = lastModifiedDates.length;
final int length = pathsLength > lastModifiedDatesLength ? lastModifiedDatesLength : pathsLength;
for (int i = 0; i < length; i++) {
String path = paths[i];
for (PathTimestamp pathTimestamp : pathTimestamps) {
try {
long lastModified = getResourceAttributes(path).getLastModified();
long oldLastModified = lastModifiedDates[i];
if (lastModified != oldLastModified) {
long currentLastModified = getResourceAttributes(pathTimestamp.path).getLastModified();
long oldLastModified = pathTimestamp.timestamp;
if (currentLastModified != oldLastModified) {
if (LOG.isLoggable(DEBUG)) {
LOG.log(DEBUG, "Resource {0} was modified at {1}, old time stamp was {2}.", path,
Instant.ofEpochMilli(lastModified), Instant.ofEpochMilli(oldLastModified));
LOG.log(DEBUG, "Resource {0} was modified at {1}, old time stamp was {2}.", pathTimestamp.path,
Instant.ofEpochMilli(currentLastModified), Instant.ofEpochMilli(oldLastModified));
}
return true;
}
} catch (NamingException e) {
LOG.log(ERROR, LogFacade.MISSING_RESOURCE, path);
LOG.log(ERROR, LogFacade.MISSING_RESOURCE, pathTimestamp.path);
return true;
}
}

try {
final int jarNamesLength = jarNames.size();
NamingEnumeration<Binding> bindings = jndiResources.listBindings(WEB_INF_LIB);
final NamingEnumeration<Binding> bindings = jndiResources.listBindings(WEB_INF_LIB);
int i = 0;
while (bindings.hasMoreElements() && i < jarNamesLength) {
NameClassPair ncPair = bindings.nextElement();
Expand All @@ -623,7 +602,6 @@ public boolean modified() {
continue;
}
if (!name.equals(jarNames.get(i))) {
// Missing JAR
LOG.log(TRACE, "JAR files changed: {0}", name);
return true;
}
Expand All @@ -635,14 +613,12 @@ public boolean modified() {
String name = ncPair.getName();
// Additional non-JAR files are allowed
if (name.endsWith(".jar") || name.endsWith(".zip")) {
// There was more JARs
LOG.log(TRACE, "Additional JARs have been added: {0}", name);
return true;
}
}
} else if (i < jarNamesLength) {
// There was less JARs
LOG.log(TRACE, "JAR files changed.");
LOG.log(TRACE, "Some JAR file was removed.");
return true;
}
} catch (NamingException | ClassCastException e) {
Expand Down Expand Up @@ -681,8 +657,9 @@ public String toString() {
sb.append("[delegate=").append(delegate);
sb.append(", context=").append(contextName);
sb.append(", status=").append(status);
sb.append(", notFound.size=").append(notFoundResources.size());
sb.append(", repositories=").append(repositoryManager);
sb.append(", notFound.size=").append(notFoundResources.size());
sb.append(", pathTimestamps.size=").append(pathTimestamps.size());
sb.append(']');
return sb.toString();
}
Expand Down Expand Up @@ -1190,12 +1167,12 @@ public void close() throws IOException {

notFoundResources.clear();
resourceEntryCache.clear();
pathTimestamps.clear();

jndiResources = null;
repositoryManager.close();
repositoryURLs = null;
lastModifiedDates = null;
paths = null;
hasExternalRepositories = false;
repositoryManager.close();

permissionList.clear();
permissionsHolder = null;
Expand Down Expand Up @@ -1738,60 +1715,40 @@ private ResourceEntry findResourceInternal(String name, String path) {
*/
private ResourceEntry findResourceInternalFromRepositories(String name, String path) {
LOG.log(TRACE, "findResourceInternalFromRepositories(name={0}, path={1})", name, path);

int contentLength = -1;
InputStream binaryStream = null;
ResourceEntry entry = null;
for (RepositoryResource repository : repositoryManager.getResources(path)) {
for (RepositoryResource repoResource : repositoryManager.getResources(path)) {
try {
final Object lookupResult = jndiResources.lookup(repository.name);
final Object lookupResult = jndiResources.lookup(repoResource.name);
final Resource resource;
if (lookupResult instanceof Resource) {
resource = (Resource) lookupResult;
} else {
resource = null;
continue;
}

// Note : Not getting an exception here means the resource was found
final ResourceEntry entry;
if (SECURITY_MANAGER == null) {
entry = createEntry(repository.file);
entry = createEntry(repoResource.file);
} else {
PrivilegedAction<ResourceEntry> dp = () -> createEntry(repository.file);
PrivilegedAction<ResourceEntry> dp = () -> createEntry(repoResource.file);
entry = AccessController.doPrivileged(dp);
}
if (resource == null || entry == null) {
continue;
}

ResourceAttributes attributes = getResourceAttributes(repository.name);
contentLength = (int) attributes.getContentLength();
final ResourceAttributes attributes = getResourceAttributes(repoResource.name);
entry.lastModified = attributes.getLastModified();

try {
binaryStream = resource.streamContent();
final int contentLength = (int) attributes.getContentLength();
try (InputStream binaryStream = resource.streamContent()) {
if (binaryStream != null) {
entry.readEntryData(name, binaryStream, contentLength, null);
}
} catch (IOException e) {
LOG.log(TRACE, "Could not read entry data for " + name, e);
return null;
}

// Register the full path for modification checking
// Note: Only syncing on a 'constant' object is needed
synchronized (ALL_PERMISSION) {
long[] newLastModifiedDates = Arrays.copyOf(lastModifiedDates, lastModifiedDates.length + 1);
newLastModifiedDates[lastModifiedDates.length] = entry.lastModified;
lastModifiedDates = newLastModifiedDates;

String[] newPaths = Arrays.copyOf(paths, paths.length + 1);
newPaths[paths.length] = repository.name;
paths = newPaths;
}
pathTimestamps.add(new PathTimestamp(repoResource.name, entry.lastModified));
return entry;
} catch (NamingException e) {
// not found, search continues.
}
}

if (entry != null && binaryStream != null) {
entry.readEntryData(name, binaryStream, contentLength, null);
}
return entry;
return null;
}


Expand Down Expand Up @@ -2090,4 +2047,15 @@ public byte[] preprocess(String resourceName, byte[] classBytes) {
private enum LifeCycleStatus {
NEW, RUNNING, CLOSED
}


private static class PathTimestamp {
final String path;
final long timestamp;

public PathTimestamp(String path, long timestamp) {
this.path = path;
this.timestamp = timestamp;
}
}
}

0 comments on commit 03bbaa5

Please sign in to comment.