Skip to content

Commit

Permalink
Fixed uncontrolled file descriptor/memory usage. This closes #6
Browse files Browse the repository at this point in the history
  • Loading branch information
krosenvold committed Jun 16, 2015
1 parent 92c0628 commit 1ddd40b
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 40 deletions.
29 changes: 17 additions & 12 deletions src/main/java/org/codehaus/plexus/archiver/jar/JarArchiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
import java.io.*;
import java.util.*;

import javax.annotation.WillClose;

import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.apache.commons.compress.parallel.InputStreamSupplier;
import org.codehaus.plexus.archiver.ArchiverException;
import org.codehaus.plexus.archiver.zip.ConcurrentJarCreator;
import org.codehaus.plexus.archiver.zip.ZipArchiver;
Expand Down Expand Up @@ -333,7 +332,8 @@ private void writeManifest( ConcurrentJarCreator zOut, Manifest manifest )
manifest.write( baos );

ByteArrayInputStream bais = new ByteArrayInputStream( baos.toByteArray() );
super.zipFile( bais, zOut, MANIFEST_NAME, System.currentTimeMillis(), null, DEFAULT_FILE_MODE, null );
super.zipFile( createInputStreamSupplier( bais ), zOut, MANIFEST_NAME, System.currentTimeMillis(), null,
DEFAULT_FILE_MODE, null );
super.initZipOutputStream( zOut );
}

Expand Down Expand Up @@ -434,21 +434,23 @@ private void createIndexList( ConcurrentJarCreator zOut )

ByteArrayInputStream bais = new ByteArrayInputStream( baos.toByteArray() );

super.zipFile( bais, zOut, INDEX_NAME, System.currentTimeMillis(), null, DEFAULT_FILE_MODE, null );
super.zipFile( createInputStreamSupplier( bais ), zOut, INDEX_NAME, System.currentTimeMillis(), null,
DEFAULT_FILE_MODE, null );
}

/**
* Overridden from Zip class to deal with manifests and index lists.
*/
protected void zipFile( @WillClose InputStream is, ConcurrentJarCreator zOut, String vPath, long lastModified, File fromArchive,
protected void zipFile( InputStreamSupplier is, ConcurrentJarCreator zOut, String vPath,
long lastModified, File fromArchive,
int mode, String symlinkDestination )
throws IOException, ArchiverException
{
if ( MANIFEST_NAME.equalsIgnoreCase( vPath ) )
{
if ( !doubleFilePass || skipWriting )
{
filesetManifest( fromArchive, is );
filesetManifest( fromArchive, is.get() );
}
}
else if ( INDEX_NAME.equalsIgnoreCase( vPath ) && index )
Expand Down Expand Up @@ -483,7 +485,7 @@ private void filesetManifest( File file, InputStream is )
manifest = getManifest( file );
}
}
else if ( ( filesetManifestConfig != null ) && filesetManifestConfig != FilesetManifestConfig.skip)
else if ( ( filesetManifestConfig != null ) && filesetManifestConfig != FilesetManifestConfig.skip )
{
// we add this to our group of fileset manifests
getLogger().debug( "Found manifest to merge in file " + file );
Expand Down Expand Up @@ -522,16 +524,17 @@ protected boolean createEmptyZip( File zipFile )
try
{
getLogger().debug( "Building MANIFEST-only jar: " + getDestFile().getAbsolutePath() );
zipArchiveOutputStream = new ZipArchiveOutputStream( bufferedOutputStream( fileOutputStream( getDestFile(), "jar" ) ));
zipArchiveOutputStream =
new ZipArchiveOutputStream( bufferedOutputStream( fileOutputStream( getDestFile(), "jar" ) ) );

zipArchiveOutputStream.setEncoding(getEncoding());
zipArchiveOutputStream.setEncoding( getEncoding() );
if ( isCompress() )
{
zipArchiveOutputStream.setMethod(ZipArchiveOutputStream.DEFLATED);
zipArchiveOutputStream.setMethod( ZipArchiveOutputStream.DEFLATED );
}
else
{
zipArchiveOutputStream.setMethod(ZipArchiveOutputStream.STORED);
zipArchiveOutputStream.setMethod( ZipArchiveOutputStream.STORED );
}
ConcurrentJarCreator ps = new ConcurrentJarCreator(Runtime.getRuntime().availableProcessors());
initZipOutputStream( ps );
Expand Down Expand Up @@ -589,7 +592,9 @@ public void reset()

public enum FilesetManifestConfig
{
skip, merge, mergewithoutmain
skip,
merge,
mergewithoutmain
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.apache.commons.compress.archivers.zip.ZipEncoding;
import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
import java.util.concurrent.ExecutionException;
import java.util.zip.CRC32;

import org.apache.commons.compress.parallel.InputStreamSupplier;
import org.codehaus.plexus.archiver.AbstractArchiver;
import org.codehaus.plexus.archiver.ArchiveEntry;
Expand All @@ -47,6 +44,8 @@
import java.io.SequenceInputStream;
import java.util.Hashtable;
import java.util.Stack;
import java.util.concurrent.ExecutionException;
import java.util.zip.CRC32;

import static org.codehaus.plexus.archiver.util.Streams.bufferedOutputStream;
import static org.codehaus.plexus.archiver.util.Streams.fileOutputStream;
Expand Down Expand Up @@ -82,7 +81,7 @@ public abstract class AbstractZipArchiver

protected final Hashtable<String, String> entries = new Hashtable<String, String>();

protected final AddedDirs addedDirs = new AddedDirs();
protected final AddedDirs addedDirs = new AddedDirs();

private static final long EMPTY_CRC = new CRC32().getValue();

Expand Down Expand Up @@ -115,7 +114,8 @@ public abstract class AbstractZipArchiver
* <p/>
* plexus-archiver chooses to round up.
* <p/>
* Java versions up to java7 round timestamp down, which means we add a heuristic value (which is slightly questionable)
* Java versions up to java7 round timestamp down, which means we add a heuristic value (which is slightly
* questionable)
* Java versions from 8 and up round timestamp up.
* s
*/
Expand Down Expand Up @@ -407,16 +407,16 @@ private void addParentDirs(ArchiveEntry archiveEntry, File baseDir, String entry

/**
* Adds a new entry to the archive, takes care of duplicates as well.
*
* @param in the stream to read data for the entry from.
* @param in the stream to read data for the entry from.
* @param zOut the stream to write to.
* @param vPath the name this entry shall have in the archive.
* @param lastModified last modification time for the entry.
* @param fromArchive the original archive we are copying this
* @param symlinkDestination
*/
@SuppressWarnings( { "JavaDoc" } )
protected void zipFile( @WillClose InputStream in, ConcurrentJarCreator zOut, String vPath, long lastModified,
protected void zipFile( InputStreamSupplier in, ConcurrentJarCreator zOut, String vPath,
long lastModified,
File fromArchive, int mode, String symlinkDestination )
throws IOException, ArchiverException
{
Expand All @@ -429,16 +429,7 @@ protected void zipFile( @WillClose InputStream in, ConcurrentJarCreator zOut, St
ZipArchiveEntry ze = new ZipArchiveEntry( vPath );
setTime( ze, lastModified );

byte[] header = new byte[4];
int read = in.read( header );

boolean compressThis = doCompress;
if ( !recompressAddedZips && isZipHeader( header ) )
{
compressThis = false;
}

ze.setMethod( compressThis ? ZipArchiveEntry.DEFLATED : ZipArchiveEntry.STORED );
ze.setMethod( doCompress ? ZipArchiveEntry.DEFLATED : ZipArchiveEntry.STORED );
ze.setUnixMode( UnixStat.FILE_FLAG | mode );

InputStream payload;
Expand All @@ -447,13 +438,12 @@ protected void zipFile( @WillClose InputStream in, ConcurrentJarCreator zOut, St
ZipEncoding enc = ZipEncodingHelper.getZipEncoding( getEncoding() );
final byte[] bytes = enc.encode( symlinkDestination ).array();
payload = new ByteArrayInputStream( bytes );
zOut.addArchiveEntry( ze, createInputStreamSupplier( payload ) );
}
else
{
payload = maybeSequence( header, read, in );
zOut.addArchiveEntry( ze, wrappedRecompressor( ze, in ) );
}
zOut.addArchiveEntry( ze, createInputStreamSupplier( payload ) );

}
}

Expand All @@ -476,7 +466,7 @@ private boolean isZipHeader( byte[] header )
* @param vPath the name this entry shall have in the archive
*/
@SuppressWarnings( { "JavaDoc" } )
protected void zipFile( ArchiveEntry entry, ConcurrentJarCreator zOut, String vPath )
protected void zipFile( final ArchiveEntry entry, ConcurrentJarCreator zOut, String vPath )
throws IOException, ArchiverException
{
final PlexusIoResource resource = entry.getResource();
Expand All @@ -487,7 +477,21 @@ protected void zipFile( ArchiveEntry entry, ConcurrentJarCreator zOut, String vP

final boolean b = entry.getResource() instanceof SymlinkDestinationSupplier;
String symlinkTarget = b ? ( (SymlinkDestinationSupplier) entry.getResource() ).getSymlinkDestination() : null;
InputStream in = entry.getInputStream();
InputStreamSupplier in = new InputStreamSupplier()
{
@Override
public InputStream get()
{
try
{
return entry.getInputStream();
}
catch ( IOException e )
{
throw new RuntimeException( e );
}
}
};
try
{
zipFile( in, zOut, vPath, resource.getLastModified(), null, entry.getMode(), symlinkTarget );
Expand Down Expand Up @@ -584,7 +588,39 @@ protected void zipDir( PlexusIoResource dir, ConcurrentJarCreator zOut, String v
}
}

private InputStreamSupplier createInputStreamSupplier( final InputStream inputStream )

private InputStreamSupplier wrappedRecompressor( final ZipArchiveEntry ze, final InputStreamSupplier other )
{

return new InputStreamSupplier()
{
public InputStream get()
{
InputStream is = other.get();
byte[] header = new byte[4];
try
{
int read = is.read( header );
boolean compressThis = doCompress;
if ( !recompressAddedZips && isZipHeader( header ) )
{
compressThis = false;
}

ze.setMethod( compressThis ? ZipArchiveEntry.DEFLATED : ZipArchiveEntry.STORED );

return maybeSequence( header, read, is );
}
catch ( IOException e )
{
throw new RuntimeException( e );
}

}
};
}

protected InputStreamSupplier createInputStreamSupplier( final InputStream inputStream )
{
return new InputStreamSupplier()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package org.codehaus.plexus.archiver.jar;

import junit.framework.TestCase;
import org.codehaus.plexus.archiver.ArchiverException;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.Random;

import junit.framework.TestCase;

public class JarArchiverTest
extends TestCase
{
Expand Down Expand Up @@ -53,12 +52,12 @@ public void testVeryLargeJar()
tmpDir.delete();
tmpDir.mkdirs();
Random rand = new Random();
for ( int i = 0; i < 15000; i++ )
for ( int i = 0; i < 45000; i++ )
{
File f = new File( tmpDir, "file" + i );
f.deleteOnExit();
FileOutputStream out = new FileOutputStream(f);
byte[] data = new byte[10240]; // 10kb per file
byte[] data = new byte[512]; // 512bytes per file
rand.nextBytes( data );
out.write( data );
out.flush();
Expand Down

0 comments on commit 1ddd40b

Please sign in to comment.