Skip to content

Avoid excessive heap utilisation due to in memory creation of md5s #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Avoid excessive heap utilisation due to in memory creation of md5s #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68b2a36
Avoid excessive heap utilisation due to in memory creation of md5s
DanielThomas Dec 4, 2015
6b0a7c3
Cleanup temporary file
DanielThomas Dec 4, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/main/java/org/vafer/jdeb/ControlBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,6 @@

package org.vafer.jdeb;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.math.BigInteger;
import java.text.ParseException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.zip.GZIPOutputStream;

import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.io.IOUtils;
Expand All @@ -43,6 +28,15 @@
import org.vafer.jdeb.utils.Utils;
import org.vafer.jdeb.utils.VariableResolver;

import java.io.*;
import java.math.BigInteger;
import java.text.ParseException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.zip.GZIPOutputStream;

/**
* Builds the control archive of the Debian package.
*/
Expand Down Expand Up @@ -71,15 +65,14 @@ class ControlBuilder {
*
* @param packageControlFile the package control file
* @param controlFiles the other control information files (maintainer scripts, etc)
* @param dataSize the size of the installed package
* @param checksums the md5 checksums of the files in the data archive
* @param output
* @return
* @throws java.io.FileNotFoundException
* @throws java.io.IOException
* @throws java.text.ParseException
*/
void buildControl(BinaryPackageControlFile packageControlFile, File[] controlFiles, List<String> conffiles, StringBuilder checksums, File output) throws IOException, ParseException {
void buildControl(BinaryPackageControlFile packageControlFile, File[] controlFiles, List<String> conffiles, File checksums, File output) throws IOException, ParseException {
final File dir = output.getParentFile();
if (dir != null && (!dir.exists() || !dir.isDirectory())) {
throw new IOException("Cannot write control file at '" + output.getAbsolutePath() + "'");
Expand Down Expand Up @@ -142,7 +135,7 @@ void buildControl(BinaryPackageControlFile packageControlFile, File[] controlFil
}

addControlEntry("control", packageControlFile.toString(), outputStream);
addControlEntry("md5sums", checksums.toString(), outputStream);
addControlEntry("md5sums", checksums, outputStream);

outputStream.close();
}
Expand Down Expand Up @@ -223,7 +216,28 @@ private static void addControlEntry(final String pName, final String pContent, f
pOutput.write(data);
pOutput.closeArchiveEntry();
}


private static void addControlEntry(final String pName, final File data, final TarArchiveOutputStream pOutput) throws IOException {
final TarArchiveEntry entry = new TarArchiveEntry("./" + pName, true);
entry.setSize(data.length());
entry.setNames("root", "root");

if (MAINTAINER_SCRIPTS.contains(pName)) {
entry.setMode(PermMapper.toMode("755"));
} else {
entry.setMode(PermMapper.toMode("644"));
}

pOutput.putArchiveEntry(entry);
InputStream input = new FileInputStream(data);
try {
IOUtils.copy(input, pOutput);
} finally {
input.close();
}
pOutput.closeArchiveEntry();
}

/**
* Tells if the specified directory is ignored by default (.svn, cvs, etc)
*
Expand Down
30 changes: 15 additions & 15 deletions src/main/java/org/vafer/jdeb/DataBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,15 @@

package org.vafer.jdeb;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarConstants;
import org.apache.commons.compress.archivers.zip.ZipEncoding;
import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
import org.apache.commons.compress.compressors.CompressorException;
import org.vafer.jdeb.utils.Utils;

import java.io.*;
import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.security.DigestOutputStream;
Expand All @@ -29,14 +34,6 @@
import java.util.Collection;
import java.util.List;

import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarConstants;
import org.apache.commons.compress.archivers.zip.ZipEncoding;
import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
import org.apache.commons.compress.compressors.CompressorException;
import org.vafer.jdeb.utils.Utils;

/**
* Builds the data archive of the Debian package.
*/
Expand Down Expand Up @@ -78,14 +75,14 @@ private void checkField(String name, int length) throws IOException {
*
* @param producers
* @param output
* @param checksums
* @param md5File
* @param compression the compression method used for the data file
* @return
* @throws java.security.NoSuchAlgorithmException
* @throws java.io.IOException
* @throws org.apache.commons.compress.compressors.CompressorException
*/
BigInteger buildData(Collection<DataProducer> producers, File output, final StringBuilder checksums, Compression compression) throws NoSuchAlgorithmException, IOException, CompressorException {
BigInteger buildData(Collection<DataProducer> producers, File output, File md5File, Compression compression) throws NoSuchAlgorithmException, IOException, CompressorException {

final File dir = output.getParentFile();
if (dir != null && (!dir.exists() || !dir.isDirectory())) {
Expand All @@ -95,6 +92,8 @@ BigInteger buildData(Collection<DataProducer> producers, File output, final Stri
final TarArchiveOutputStream tarOutputStream = new TarArchiveOutputStream(compression.toCompressedOutputStream(new FileOutputStream(output)));
tarOutputStream.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);

final FileWriter md5FileWriter = new FileWriter(md5File);

final MessageDigest digest = MessageDigest.getInstance("MD5");

final Total dataSize = new Total();
Expand Down Expand Up @@ -160,7 +159,7 @@ public void onEachFile(InputStream input, TarArchiveEntry entry) throws IOExcept
);

// append to file md5 list, two spaces to be compatible with GNU coreutils md5sum
checksums.append(md5).append(" ").append(entry.getName()).append('\n');
md5FileWriter.append(md5).append(" ").append(entry.getName()).append('\n');
}

@Override
Expand Down Expand Up @@ -263,6 +262,7 @@ private void createParentDirectories( String filename, String user, int uid, Str
}
} finally {
tarOutputStream.close();
md5FileWriter.close();
}

console.debug("Total size: " + dataSize);
Expand Down
33 changes: 15 additions & 18 deletions src/main/java/org/vafer/jdeb/DebMaker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@
import org.vafer.jdeb.utils.Utils;
import org.vafer.jdeb.utils.VariableResolver;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.*;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -463,15 +458,16 @@ public BinaryPackageControlFile createDeb(Compression compression) throws Packag
public BinaryPackageControlFile createSignedDeb(Compression compression, final PGPSignatureGenerator signatureGenerator, PGPSigner signer ) throws PackagingException {
File tempData = null;
File tempControl = null;
File tempMd5 = null;

try {
tempData = File.createTempFile("deb", "data");
tempControl = File.createTempFile("deb", "control");
tempMd5 = File.createTempFile("deb", "md5");

console.debug("Building data");
DataBuilder dataBuilder = new DataBuilder(console);
StringBuilder md5s = new StringBuilder();
BigInteger size = dataBuilder.buildData(dataProducers, tempData, md5s, compression);
BigInteger size = dataBuilder.buildData(dataProducers, tempData, tempMd5, compression);

console.info("Building conffiles");
List<String> tempConffiles = populateConffiles(conffilesProducers);
Expand All @@ -495,7 +491,7 @@ public BinaryPackageControlFile createSignedDeb(Compression compression, final P
packageControlFile.set("Homepage", homepage);
}

controlBuilder.buildControl(packageControlFile, control.listFiles(), tempConffiles , md5s, tempControl);
controlBuilder.buildControl(packageControlFile, control.listFiles(), tempConffiles , tempMd5, tempControl);

if (!packageControlFile.isValid()) {
throw new PackagingException("Control file fields are invalid " + packageControlFile.invalidFields() +
Expand Down Expand Up @@ -560,15 +556,16 @@ public BinaryPackageControlFile createSignedDeb(Compression compression, final P
} catch (Exception e) {
throw new PackagingException("Could not create deb package", e);
} finally {
if (tempData != null) {
if (!tempData.delete()) {
console.warn("Could not delete the temporary file " + tempData);
}
}
if (tempControl != null) {
if (!tempControl.delete()) {
console.warn("Could not delete the temporary file " + tempControl);
}
deleteTempFile(tempData);
deleteTempFile(tempControl);
deleteTempFile(tempMd5);
}
}

private void deleteTempFile(File tempFile) {
if (tempFile != null) {
if (!tempFile.delete()) {
console.warn("Could not delete the temporary file " + tempFile);
}
}
}
Expand Down
20 changes: 11 additions & 9 deletions src/test/java/org/vafer/jdeb/DataBuilderTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@

package org.vafer.jdeb;

import java.io.File;
import java.io.FileInputStream;
import java.util.Arrays;

import junit.framework.TestCase;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.apache.commons.io.IOUtils;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.types.FileSet;
import org.vafer.jdeb.producers.DataProducerFile;
import org.vafer.jdeb.producers.DataProducerFileSet;

import java.io.File;
import java.io.FileInputStream;
import java.util.Arrays;

public class DataBuilderTestCase extends TestCase {

/**
Expand All @@ -45,12 +46,13 @@ public void testBuildDataWithFileSet() throws Exception {
fileset.setIncludes("**/*");
fileset.setProject(project);

StringBuilder md5s = new StringBuilder();
builder.buildData(Arrays.asList((DataProducer) new DataProducerFileSet(fileset)), new File("target/data.tar"), md5s, Compression.GZIP);
File md5File = File.createTempFile("deb", "md5");
builder.buildData(Arrays.asList((DataProducer) new DataProducerFileSet(fileset)), new File("target/data.tar"), md5File, Compression.GZIP);

String md5s = IOUtils.toString(md5File.toURI());
assertTrue("empty md5 file", md5s.length() > 0);
assertFalse("windows path separator found", md5s.indexOf("\\") != -1);
assertTrue("two spaces between md5 and file", md5s.toString().equals("8bc944dbd052ef51652e70a5104492e3 ./test/testfile\n"));
assertFalse("windows path separator found", md5s.contains("\\"));
assertTrue("two spaces between md5 and file", md5s.equals("8bc944dbd052ef51652e70a5104492e3 ./test/testfile\n"));
}

public void testCreateParentDirectories() throws Exception {
Expand All @@ -63,7 +65,7 @@ public void testCreateParentDirectories() throws Exception {

DataProducer producer = new DataProducerFile(new File("pom.xml"), "/usr/share/myapp/pom.xml", null, null, null);

builder.buildData(Arrays.asList(producer), archive, new StringBuilder(), Compression.NONE);
builder.buildData(Arrays.asList(producer), archive, File.createTempFile("deb", "md5"), Compression.NONE);

int count = 0;
TarArchiveInputStream in = null;
Expand Down