Skip to content

Commit

Permalink
Tracked down and fixed a file that wasn't closed.
Browse files Browse the repository at this point in the history
  • Loading branch information
@thobe
thobe committed Mar 12, 2012
1 parent cb4e4fb commit df1dfc0
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 39 deletions.
2 changes: 2 additions & 0 deletions kernel/src/main/java/org/neo4j/kernel/DefaultFileSystemAbstraction.java
View file
Expand Up @@ -24,6 +24,7 @@
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.channels.FileChannel;

import org.neo4j.kernel.impl.nioneo.store.FileLock;
import org.neo4j.kernel.impl.nioneo.store.FileSystemAbstraction;
import org.neo4j.kernel.impl.util.FileUtils;
Expand All @@ -37,6 +38,7 @@ public class DefaultFileSystemAbstraction
@Override
public FileChannel open( String fileName, String mode ) throws IOException
{
// Returning only the channel is ok, because the channel, when close()d will close its parent File.
return new RandomAccessFile( fileName, mode ).getChannel();
}

Expand Down
31 changes: 16 additions & 15 deletions kernel/src/main/java/org/neo4j/kernel/impl/nioneo/store/NeoStore.java
View file
Expand Up @@ -48,7 +48,7 @@ public interface Configuration
{
int relationship_grab_size(int defaultRelGrabSize);
}

public static final String TYPE_DESCRIPTOR = "NeoStore";

/*
Expand All @@ -68,9 +68,9 @@ public interface Configuration
private long lastCommittedTx = -1;

private final int REL_GRAB_SIZE;
private String fileName;
private Configuration conf;
private LastCommittedTxIdSetter lastCommittedTxIdSetter;
private final String fileName;
private final Configuration conf;
private final LastCommittedTxIdSetter lastCommittedTxIdSetter;

public NeoStore(String fileName, Configuration conf,
LastCommittedTxIdSetter lastCommittedTxIdSetter,
Expand All @@ -94,7 +94,7 @@ public NeoStore(String fileName, Configuration conf,
* thereafter so this missing record doesn't trigger an upgrade of the neostore file and so any
* unclean shutdown on such a db with 1.5.M02 < neo4j version <= 1.6.M02 would make that
* db unable to start for that version with a "Mismatching store version found" exception.
*
*
* This will make a cleanly shut down 1.5.M02, then started and cleanly shut down with 1.6.M03 (or higher)
* successfully add the missing record.
*/
Expand Down Expand Up @@ -158,12 +158,12 @@ protected void checkVersion()
+ getStorageFileName(), e );
}
}

@Override
protected void verifyFileSizeAndTruncate() throws IOException
{
super.verifyFileSizeAndTruncate();

/* MP: 2011-11-23
* A little silent upgrade for the "next prop" record. It adds one record last to the neostore file.
* It's backwards compatible, that's why it can be a silent and automatic upgrade.
Expand Down Expand Up @@ -321,10 +321,10 @@ public static long getTxId( FileSystemAbstraction fs, String storeDir )

private static long getRecord( FileSystemAbstraction fs, String storeDir, long recordPosition )
{
RandomAccessFile file = null;
FileChannel channel = null;
try
{
FileChannel channel = fs.open( storeDir, "rw" );
channel = fs.open( storeDir, "rw" );
/*
* We have to check size, because the store version
* field was introduced with 1.5, so if there is a non-clean
Expand All @@ -349,7 +349,7 @@ private static long getRecord( FileSystemAbstraction fs, String storeDir, long r
{
try
{
if ( file != null ) file.close();
if ( channel != null ) channel.close();
}
catch ( IOException e )
{
Expand Down Expand Up @@ -479,7 +479,7 @@ private void setRecord( long id, long value )
releaseWindow( window );
}
}

public long getStoreVersion()
{
return getRecord( 4 );
Expand All @@ -489,17 +489,17 @@ public void setStoreVersion( long version )
{
setRecord( 4, version );
}

public long getGraphNextProp()
{
return getRecord( 5 );
}

public void setGraphNextProp( long propId )
{
setRecord( 5, propId );
}

/**
* Returns the node store.
*
Expand Down Expand Up @@ -606,6 +606,7 @@ public void logVersions( StringLogger.LineLogger msgLog)
stringLogger.flush();
}

@Override
public void logIdUsage( StringLogger.LineLogger msgLog )
{
msgLog.logLine( "Id usage:" );
Expand All @@ -615,7 +616,7 @@ public void logIdUsage( StringLogger.LineLogger msgLog )
propStore.logIdUsage( msgLog );
stringLogger.flush();
}

public NeoStoreRecord asRecord()
{
NeoStoreRecord result = new NeoStoreRecord();
Expand Down
11 changes: 7 additions & 4 deletions .../test/java/org/neo4j/kernel/impl/nioneo/store/IdGeneratorRebuildFailureEmulationTest.java
View file
Expand Up @@ -56,7 +56,7 @@

@RunWith( Suite.class )
@SuiteClasses( { IdGeneratorRebuildFailureEmulationTest.FailureBeforeRebuild.class,
IdGeneratorRebuildFailureEmulationTest.FailureDuringRebuild.class } )
IdGeneratorRebuildFailureEmulationTest.FailureDuringRebuild.class } )
public class IdGeneratorRebuildFailureEmulationTest
{
@RunWith( JUnit4.class )
Expand Down Expand Up @@ -152,7 +152,7 @@ public void initialize()
}

@After
public void verifyAndDispose()
public void verifyAndDispose() throws Exception
{
try
{
Expand All @@ -162,7 +162,7 @@ public void verifyAndDispose()
}
finally
{
if ( fs != null ) fs.dispose();
if ( fs != null ) fs.disposeAndAssertNoOpenFiles();
fs = null;
}
}
Expand Down Expand Up @@ -230,8 +230,11 @@ private int readProperties( PropertyContainer entity )

private static class FileSystem extends EphemeralFileSystemAbstraction
{
void dispose()
void disposeAndAssertNoOpenFiles() throws Exception
{
//Collection<String> open = openFiles();
//assertTrue( "Open files: " + open, open.isEmpty() );
assertNoOpenFiles();
super.shutdown();
}

Expand Down
121 changes: 101 additions & 20 deletions kernel/src/test/java/org/neo4j/test/impl/EphemeralFileSystemAbstraction.java
View file
Expand Up @@ -22,14 +22,20 @@
import java.io.IOException;
import java.lang.ref.Reference;
import java.lang.ref.SoftReference;
import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.nio.MappedByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
Expand All @@ -38,10 +44,14 @@
import static java.lang.Math.max;
import static java.lang.Math.min;

import org.junit.runners.model.MultipleFailureException;
import org.neo4j.helpers.collection.PrefetchingIterator;
import org.neo4j.kernel.Lifecycle;
import org.neo4j.kernel.impl.nioneo.store.FileLock;
import org.neo4j.kernel.impl.nioneo.store.FileSystemAbstraction;

import static org.neo4j.helpers.collection.IteratorUtil.loop;

public class EphemeralFileSystemAbstraction implements FileSystemAbstraction, Lifecycle
{
private final Map<String, EphemeralFileData> files = new HashMap<String, EphemeralFileData>();
Expand Down Expand Up @@ -77,16 +87,42 @@ protected void finalize() throws Throwable
super.finalize();
}

public void assertNoOpenFiles() throws Exception
{
List<Throwable> open = new ArrayList<Throwable>();
for ( EphemeralFileData file : files.values() )
{
for ( EphemeralFileChannel channel : loop( file.getOpenChannels() ) )
{
open.add( channel.openedAt );
}
}
if (!open.isEmpty())
{
if (open.size() == 1) throw (FileStillOpenException) open.get( 0 );
throw new MultipleFailureException( open );
}
}

@SuppressWarnings( "serial" )
private static class FileStillOpenException extends Exception
{
FileStillOpenException( String filename )
{
super( "File still open: [" + filename + "]" );
}
}

private void free(EphemeralFileData file)
{
if (file != null) file.fileAsBuffer.free();
}

@Override
public synchronized FileChannel open(String fileName, String mode) throws IOException
public synchronized FileChannel open( String fileName, String mode ) throws IOException
{
EphemeralFileData data = files.get(fileName);
return data != null ? new EphemeralFileChannel( data ) : create( fileName );
EphemeralFileData data = files.get( fileName );
return data != null ? new EphemeralFileChannel( data, new FileStillOpenException( fileName ) ) : create( fileName );
}

@Override
Expand Down Expand Up @@ -114,7 +150,7 @@ public synchronized FileChannel create(String fileName) throws IOException
{
EphemeralFileData data = new EphemeralFileData();
free(files.put(fileName, data));
return new EphemeralFileChannel( data );
return new EphemeralFileChannel( data, new FileStillOpenException( fileName ) );
}

@Override
Expand Down Expand Up @@ -148,12 +184,15 @@ public boolean renameFile(String from, String to) throws IOException

private static class EphemeralFileChannel extends FileChannel
{
final FileStillOpenException openedAt;
private final EphemeralFileData data;
long position = 0;

EphemeralFileChannel( EphemeralFileData data )
EphemeralFileChannel( EphemeralFileData data, FileStillOpenException opened )
{
this.data = data;
this.openedAt = opened;
data.open( this );
}

@Override
Expand Down Expand Up @@ -277,14 +316,15 @@ public java.nio.channels.FileLock tryLock( long position, long size, boolean sha
@Override
protected void implCloseChannel() throws IOException
{
data.close();
data.close( this );
}
}

private static class EphemeralFileData
{
private final DynamicByteBuffer fileAsBuffer = new DynamicByteBuffer();
private final byte[] scratchPad = new byte[1024];
private final Collection<WeakReference<EphemeralFileChannel>> channels = new LinkedList<WeakReference<EphemeralFileChannel>>();
private int size;
private int locked;

Expand All @@ -306,6 +346,53 @@ int read( EphemeralFileChannel fc, ByteBuffer dst )
return available; // return how much data was read
}

void open( EphemeralFileChannel channel )
{
channels.add( new WeakReference<EphemeralFileChannel>( channel ) );
}

void close( EphemeralFileChannel channel )
{
locked = 0; // Regular file systems seems to release all file locks when closed...
for ( Iterator<EphemeralFileChannel> iter = getOpenChannels(); iter.hasNext(); )
{
if ( iter.next() == channel )
{
iter.remove();
}
}
}

Iterator<EphemeralFileChannel> getOpenChannels()
{
final Iterator<WeakReference<EphemeralFileChannel>> refs = channels.iterator();
return new PrefetchingIterator<EphemeralFileChannel>()
{
@Override
protected EphemeralFileChannel fetchNextOrNull()
{
while ( refs.hasNext() )
{
EphemeralFileChannel channel = refs.next().get();
if ( channel != null ) return channel;
refs.remove();
}
return null;
}

@Override
public void remove()
{
refs.remove();
}
};
}

boolean isOpen()
{
return getOpenChannels().hasNext();
}

int write(EphemeralFileChannel fc, ByteBuffer src)
{
int wanted = src.limit();
Expand Down Expand Up @@ -347,37 +434,31 @@ boolean lock()
{
return locked == 0;
}

void close()
{
locked = 0;
}
}

private static class EphemeralFileLock extends java.nio.channels.FileLock
{
private final EphemeralFileData data;
private boolean released;
private EphemeralFileData file;

EphemeralFileLock(EphemeralFileChannel channel, EphemeralFileData data)
EphemeralFileLock(EphemeralFileChannel channel, EphemeralFileData file)
{
super(channel, 0, Long.MAX_VALUE, false);
this.data = data;
data.locked++;
this.file = file;
file.locked++;
}

@Override
public boolean isValid()
{
return !released;
return file != null;
}

@Override
public void release() throws IOException
{
if (released || data.locked == 0) return;
data.locked--;
released = true;
if (file == null || file.locked == 0) return;
file.locked--;
file = null;
}
}

Expand Down

0 comments on commit df1dfc0

Please sign in to comment.