From 4efd5a37d8489463f20a9865ba7211db0b31f4c5 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Mon, 21 Mar 2022 17:20:23 +0100 Subject: [FileTransfer] Log stack traces on error, some OOM handling Try smaller buffers for uploading on OOM errors. Log error reason instead of just sending to peer, and also print a stack trace if applicable. --- .../java/org/openslx/filetransfer/Downloader.java | 17 +++++++++++++++-- .../java/org/openslx/filetransfer/Transfer.java | 13 ++++++++++++- .../java/org/openslx/filetransfer/Uploader.java | 21 ++++++++++++++++----- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openslx/filetransfer/Downloader.java b/src/main/java/org/openslx/filetransfer/Downloader.java index 13bd95c..b2dbbd1 100644 --- a/src/main/java/org/openslx/filetransfer/Downloader.java +++ b/src/main/java/org/openslx/filetransfer/Downloader.java @@ -116,6 +116,7 @@ public class Downloader extends Transfer compressed += compressedLength; uncompressed += decompressedLength; if ( decompressedLength > len ) { + // TODO: Partial reads with buffering, if remote payload is larger than our buffer throw new RuntimeException( "This should never happen! ;)" ); } if ( decompressedLength == compressedLength ) { @@ -168,6 +169,19 @@ public class Downloader extends Transfer } FileRange requestedRange; try { + byte[] incoming = new byte[ 500000 ]; + /* TODO once the Lz4InputStream can handle small buffer sizes / partial reads + for ( int bufsiz = 600; bufsiz >= 100 && incoming == null; bufsiz -= 100 ) { + try { + incoming = new byte[ bufsiz * 1024 ]; + } catch ( OutOfMemoryError e ) { + } + } + if ( incoming == null ) { + log.error( "Could not allocate buffer for receiving." ); + return false; + } + */ while ( ( requestedRange = rangeCallback.get() ) != null ) { if ( requestedRange.startOffset < 0 || requestedRange.startOffset >= requestedRange.endOffset ) { log.error( "Callback supplied bad range (" + requestedRange.startOffset + " to " + requestedRange.endOffset + ")" ); @@ -197,7 +211,6 @@ public class Downloader extends Transfer int chunkLength = requestedRange.getLength(); // If the uploader sets the COMPRESS field, assume compressed chunk InputStream inStream = meta.peerWantsCompression() ? compressedIn : dataFromServer; - byte[] incoming = new byte[ 500000 ]; // 500kb int hasRead = 0; while ( hasRead < chunkLength ) { int ret; @@ -208,7 +221,7 @@ public class Downloader extends Transfer return false; } } catch ( IOException e ) { - log.error( "Could not read payload from socket" ); + log.error( "Could not read payload from socket", e ); sendErrorCode( "payload read error" ); return false; } diff --git a/src/main/java/org/openslx/filetransfer/Transfer.java b/src/main/java/org/openslx/filetransfer/Transfer.java index c396d6e..933e63e 100644 --- a/src/main/java/org/openslx/filetransfer/Transfer.java +++ b/src/main/java/org/openslx/filetransfer/Transfer.java @@ -218,12 +218,18 @@ public abstract class Transfer * */ protected void close( String error, UploadStatusCallback callback, boolean sendToPeer ) + { + close( error, callback, sendToPeer, null ); + } + + protected void close( String error, UploadStatusCallback callback, boolean sendToPeer, Exception e ) { if ( error != null ) { if ( sendToPeer ) sendErrorCode( error ); if ( callback != null ) callback.uploadError( error ); + log.info( "Closing with error '" + error + "'", e ); } synchronized ( transferSocket ) { safeClose( dataFromServer, outStream, transferSocket ); @@ -232,7 +238,12 @@ public abstract class Transfer protected void close( String error ) { - close( error, null, false ); + close( error, null ); + } + + protected void close( String error, Exception e ) + { + close( error, null, false, e ); } public void cancel() diff --git a/src/main/java/org/openslx/filetransfer/Uploader.java b/src/main/java/org/openslx/filetransfer/Uploader.java index 73445aa..ed6e972 100644 --- a/src/main/java/org/openslx/filetransfer/Uploader.java +++ b/src/main/java/org/openslx/filetransfer/Uploader.java @@ -136,6 +136,18 @@ public class Uploader extends Transfer this.close( "Could not open given file for reading.", callback, true ); return false; } + byte[] data = null; + // Cannot go above 500000 for backwards compat + for ( int bufsiz = 500; bufsiz >= 100 && data == null; bufsiz -= 100 ) { + try { + data = new byte[ bufsiz * 1000 ]; + } catch ( OutOfMemoryError e ) { + } + } + if ( data == null ) { + this.close( "Could not allocate buffer for reading.", callback, true ); + return false; + } while ( !Thread.currentThread().isInterrupted() ) { // Loop as long as remote peer is requesting chunks from this file // Read meta data of remote peer - either new range, or it's telling us it's done MetaData meta = readMetaData(); @@ -158,14 +170,14 @@ public class Uploader extends Transfer return false; } } catch ( IOException e ) { - this.close( "Could not get current length of file " + filename, callback, false ); + this.close( "Could not get current length of file " + filename, callback, false, e ); return false; } // Seek to requested chunk try { file.seek( requestedRange.startOffset ); } catch ( IOException e ) { - this.close( "Could not seek to start of requested range in given file (" + requestedRange.startOffset + ")", callback, true ); + this.close( "Could not seek to start of requested range in given file (" + requestedRange.startOffset + ")", callback, true, e ); return false; } // Send confirmation of range and compression mode we're about to send @@ -185,7 +197,6 @@ public class Uploader extends Transfer return false; } // Finally send requested chunk - byte[] data = new byte[ 500000 ]; // 500kb int hasRead = 0; int length = requestedRange.getLength(); while ( hasRead < length ) { @@ -193,7 +204,7 @@ public class Uploader extends Transfer try { ret = file.read( data, 0, Math.min( length - hasRead, data.length ) ); } catch ( IOException e ) { - this.close( "Error reading from file ", callback, true ); + this.close( "Error reading from file ", callback, true, e ); return false; } if ( ret == -1 ) { @@ -204,7 +215,7 @@ public class Uploader extends Transfer try { outStr.write( data, 0, ret ); } catch ( IOException e ) { - this.close( "Sending payload failed" ); + this.close( "Sending payload failed", e ); return false; } if ( callback != null ) -- cgit v1.2.3-55-g7522