Skip to content

Commit

Permalink
Trace causes of page cursor errors from file closed
Browse files Browse the repository at this point in the history
Closing a paged file will now record its stack trace in an exception, and if
a page cursor runs into trouble because it is operating on a file that has been
unmapped, then the IllegalStateException will now also contain the file closed
trace exception as a suppressed exception. This allows us to see where and why
a file was closed, and give us clues as to why we still have a cursor on such
a file.
  • Loading branch information
chrisvest committed Jan 5, 2017
1 parent 1fc3c0a commit 2b5504b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 8 deletions.
Expand Up @@ -71,6 +71,9 @@ final class MuninnPagedFile implements PagedFile
// Guarded by the monitor lock on MuninnPageCache (map and unmap)
private boolean deleteOnClose;

// Used to trace the causes of any exceptions from getLastPageId.
private volatile Exception closeStackTrace;

/**
* The header state includes both the reference count of the PagedFile – 15 bits – and the ID of the last page in
* the file – 48 bits, plus an empty file marker bit. Because our pages are usually 2^13 bytes, this means that we
Expand Down Expand Up @@ -199,6 +202,10 @@ public WritableByteChannel openWritableByteChannel() throws IOException

void closeSwapper() throws IOException
{
// We don't set closeStackTrace in close(), because the reference count may keep the file open.
// But if we get here, to close the swapper, then we are definitely unmapping!
closeStackTrace = new Exception( "tracing paged file closing" );

if ( !deleteOnClose )
{
swapper.close();
Expand Down Expand Up @@ -377,7 +384,14 @@ public long getLastPageId()
long state = getHeaderState();
if ( refCountOf( state ) == 0 )
{
throw new IllegalStateException( "File has been unmapped: " + file().getPath() );
String msg = "File has been unmapped: " + file().getPath();
IllegalStateException exception = new IllegalStateException( msg );
Exception closedBy = closeStackTrace;
if ( closedBy != null )
{
exception.addSuppressed( closedBy );
}
throw exception;
}
return state & headerStateLastPageIdMask;
}
Expand Down
Expand Up @@ -29,6 +29,8 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.ReadableByteChannel;
Expand Down Expand Up @@ -2420,31 +2422,55 @@ public void closeOnPageCacheMustThrowIfFilesAreStillMapped() throws IOException
}
}

@Test( timeout = SHORT_TIMEOUT_MILLIS, expected = IllegalStateException.class )
@Test( timeout = SHORT_TIMEOUT_MILLIS )
public void pagedFileIoMustThrowIfFileIsUnmapped() throws IOException
{
getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL );

PagedFile pagedFile = pageCache.map( file( "a" ), filePageSize );
pagedFile.close();
closeThisPagedFile( pagedFile );

try ( PageCursor cursor = pagedFile.io( 0, PF_SHARED_WRITE_LOCK ) )
{
cursor.next(); // This should throw
fail( "cursor.next() on unmapped file did not throw" );
try
{
cursor.next();
fail( "cursor.next() on unmapped file did not throw" );
}
catch ( IllegalStateException e )
{
StringWriter out = new StringWriter();
e.printStackTrace( new PrintWriter( out ) );
assertThat( out.toString(), containsString( "closeThisPagedFile" ) );
}
}
}

@Test( timeout = SHORT_TIMEOUT_MILLIS, expected = IllegalStateException.class )
protected void closeThisPagedFile( PagedFile pagedFile ) throws IOException
{
pagedFile.close();
}

@Test( timeout = SHORT_TIMEOUT_MILLIS )
public void writeLockedPageCursorNextMustThrowIfFileIsUnmapped() throws IOException
{
getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL );

PagedFile pagedFile = pageCache.map( file( "a" ), filePageSize );
PageCursor cursor = pagedFile.io( 0, PF_SHARED_WRITE_LOCK );
pagedFile.close();
closeThisPagedFile( pagedFile );

cursor.next();
try
{
cursor.next();
fail( "cursor.next() on unmapped file did not throw" );
}
catch ( IllegalStateException e )
{
StringWriter out = new StringWriter();
e.printStackTrace( new PrintWriter( out ) );
assertThat( out.toString(), containsString( "closeThisPagedFile" ) );
}
}

@Test( timeout = SHORT_TIMEOUT_MILLIS, expected = IllegalStateException.class )
Expand Down

0 comments on commit 2b5504b

Please sign in to comment.