瀏覽代碼

Do not initialize fields to default values.

Gert Driesen 8 年之前
父節點
當前提交
54fbd8f5ba

+ 99 - 0
src/Renci.SshNet.Tests/Classes/Sftp/SftpFileReaderTest_DisposeShouldUnblockReadAndReadAhead.cs

@@ -0,0 +1,99 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using Renci.SshNet.Abstractions;
+using Renci.SshNet.Sftp;
+using System;
+using System.Diagnostics;
+using System.Threading;
+using BufferedRead = Renci.SshNet.Sftp.SftpFileReader.BufferedRead;
+
+namespace Renci.SshNet.Tests.Classes.Sftp
+{
+    [TestClass]
+    public class SftpFileReaderTest_DisposeShouldUnblockReadAndReadAhead : SftpFileReaderTestBase
+    {
+        private const int ChunkLength = 32 * 1024;
+
+        private MockSequence _seq;
+        private byte[] _handle;
+        private int _fileSize;
+        private SftpFileReader _reader;
+        private ObjectDisposedException _actualException;
+
+        protected override void SetupData()
+        {
+            var random = new Random();
+
+            _handle = CreateByteArray(random, 5);
+            _fileSize = 5000;
+        }
+
+        protected override void SetupMocks()
+        {
+            _seq = new MockSequence();
+
+            SftpSessionMock.InSequence(_seq)
+                           .Setup(p => p.BeginRead(_handle, 0, ChunkLength, It.IsNotNull<AsyncCallback>(), It.IsAny<BufferedRead>()))
+                           .Returns((SftpReadAsyncResult)null);
+            SftpSessionMock.InSequence(_seq).Setup(p => p.RequestClose(_handle));
+        }
+
+        protected override void Arrange()
+        {
+            base.Arrange();
+
+            _reader = new SftpFileReader(_handle, SftpSessionMock.Object, ChunkLength, 1, _fileSize);
+        }
+
+        protected override void Act()
+        {
+            ThreadAbstraction.ExecuteThread(() =>
+            {
+                Thread.Sleep(500);
+                _reader.Dispose();
+            });
+
+            try
+            {
+                _reader.Read();
+                Assert.Fail();
+            }
+            catch (ObjectDisposedException ex)
+            {
+                _actualException = ex;
+            }
+        }
+
+        [TestMethod]
+        public void ReadShouldHaveThrownObjectDisposedException()
+        {
+            Assert.IsNotNull(_actualException);
+            Assert.AreEqual(typeof(SftpFileReader).FullName, _actualException.ObjectName);
+        }
+
+        [TestMethod]
+        public void ReadAfterDisposeShouldThrowObjectDisposedException()
+        {
+            try
+            {
+                _reader.Read();
+                Assert.Fail();
+            }
+            catch (ObjectDisposedException ex)
+            {
+                Assert.IsNull(ex.InnerException);
+                Assert.AreEqual(typeof(SftpFileReader).FullName, ex.ObjectName);
+            }
+        }
+
+        [TestMethod]
+        public void DisposeShouldCompleteImmediately()
+        {
+            var stopwatch = Stopwatch.StartNew();
+            _reader.Dispose();
+            stopwatch.Stop();
+
+            Assert.IsTrue(stopwatch.ElapsedMilliseconds < 200, "Dispose took too long to complete: " + stopwatch.ElapsedMilliseconds);
+        }
+    }
+}

+ 1 - 0
src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj

@@ -341,6 +341,7 @@
     <Compile Include="Classes\SftpClientTest_Dispose_Disposed.cs" />
     <Compile Include="Classes\SftpClientTest_Finalize_Connected.cs" />
     <Compile Include="Classes\Sftp\SftpFileReaderTestBase.cs" />
+    <Compile Include="Classes\Sftp\SftpFileReaderTest_DisposeShouldUnblockReadAndReadAhead.cs" />
     <Compile Include="Classes\Sftp\SftpFileReaderTest_LastChunkBeforeEofIsComplete.cs" />
     <Compile Include="Classes\Sftp\SftpFileReaderTest_LastChunkBeforeEofIsPartial.cs" />
     <Compile Include="Classes\Sftp\SftpFileReaderTest_PreviousChunkIsIncompleteAndEofIsNotReached.cs" />

+ 49 - 0
src/Renci.SshNet/Common/SemaphoreLight.cs

@@ -76,5 +76,54 @@ namespace Renci.SshNet.Common
                 Monitor.Pulse(_lock);
             }
         }
+
+        /// <summary>
+        /// Blocks the current thread until it can either enter the <see cref="SemaphoreLight"/>,
+        /// or until the specified timeout has expired.
+        /// </summary>
+        /// <param name="timeout"></param>
+        /// <returns></returns>
+        public bool Wait(TimeSpan timeout)
+        {
+            var timeoutInMilliseconds = timeout.TotalMilliseconds;
+            if (timeoutInMilliseconds < -1d || timeoutInMilliseconds > int.MaxValue)
+                throw new ArgumentOutOfRangeException("timeout", "The timeout must represent a value between -1 and Int32.MaxValue, inclusive.");
+
+            lock (_lock)
+            {
+                if (timeoutInMilliseconds == -1)
+                {
+                    while (_currentCount < 1)
+                        Monitor.Wait(_lock);
+                }
+                else
+                {
+                    if (_currentCount < 1)
+                    {
+                        var remainingTimeInMilliseconds = (int) timeoutInMilliseconds;
+                        var startTicks = Environment.TickCount;
+
+                        while (_currentCount < 1)
+                        {
+                            if (!Monitor.Wait(_lock, remainingTimeInMilliseconds))
+                            {
+                                return false;
+                            }
+
+                            var elapsed = Environment.TickCount - startTicks;
+                            remainingTimeInMilliseconds -= elapsed;
+                            if (remainingTimeInMilliseconds < 0)
+                                return false;
+                        }
+                    }
+                }
+
+                _currentCount--;
+
+                Monitor.Pulse(_lock);
+
+                return true;
+            }
+        }
     }
 }

+ 1 - 5
src/Renci.SshNet/IServiceFactory.cs

@@ -58,10 +58,6 @@ namespace Renci.SshNet
         /// <exception cref="SshConnectionException">No key exchange algorithm is supported by both client and server.</exception>
         IKeyExchange CreateKeyExchange(IDictionary<string, Type> clientAlgorithms, string[] serverAlgorithms);
 
-        ISftpFileReader CreateSftpFileReader(byte[] handle,
-                                             ISftpSession sftpSession,
-                                             uint chunkSize,
-                                             int maxPendingReads,
-                                             long? fileSize);
+        ISftpFileReader CreateSftpFileReader(string fileName, ISftpSession sftpSession, uint bufferSize);
     }
 }

+ 21 - 2
src/Renci.SshNet/ServiceFactory.cs

@@ -97,9 +97,28 @@ namespace Renci.SshNet
             return keyExchangeAlgorithmType.CreateInstance<IKeyExchange>();
         }
 
-        public ISftpFileReader CreateSftpFileReader(byte[] handle, ISftpSession sftpSession, uint chunkSize, int maxPendingReads, long? fileSize)
+        public ISftpFileReader CreateSftpFileReader(string fileName, ISftpSession sftpSession, uint bufferSize)
         {
-            return new SftpFileReader(handle, sftpSession, chunkSize, maxPendingReads, fileSize);
+            var handle = sftpSession.RequestOpen(fileName, Flags.Read);
+
+            long? fileSize;
+            int maxPendingReads;
+
+            var chunkSize = sftpSession.CalculateOptimalReadLength(bufferSize);
+
+            var fileAttributes = sftpSession.RequestFStat(handle, true);
+            if (fileAttributes == null)
+            {
+                fileSize = null;
+                maxPendingReads = 5;
+            }
+            else
+            {
+                fileSize = fileAttributes.Size;
+                maxPendingReads = Math.Min(10, (int)Math.Ceiling((double)fileAttributes.Size / chunkSize) + 1);
+            }
+
+            return sftpSession.CreateFileReader(handle, sftpSession, chunkSize, maxPendingReads, fileSize);
         }
     }
 }

+ 11 - 1
src/Renci.SshNet/Sftp/ISftpSession.cs

@@ -48,6 +48,16 @@ namespace Renci.SshNet.Sftp
         /// </returns>
         SftpFileAttributes RequestFStat(byte[] handle, bool nullOnError);
 
+        /// <summary>
+        /// Performs SSH_FXP_STAT request.
+        /// </summary>
+        /// <param name="path">The path.</param>
+        /// <param name="nullOnError">if set to <c>true</c> returns null instead of throwing an exception.</param>
+        /// <returns>
+        /// File attributes
+        /// </returns>
+        SftpFileAttributes RequestStat(string path, bool nullOnError = false);
+
         /// <summary>
         /// Performs SSH_FXP_LSTAT request.
         /// </summary>
@@ -224,6 +234,6 @@ namespace Renci.SshNet.Sftp
         /// </remarks>
         uint CalculateOptimalWriteLength(uint bufferSize, byte[] handle);
 
-        ISftpFileReader CreateFileReader(string fileName, uint bufferSize);
+        ISftpFileReader CreateFileReader(byte[] handle, ISftpSession sftpSession, uint chunkSize, int maxPendingReads, long? fileSize);
     }
 }

+ 38 - 5
src/Renci.SshNet/Sftp/SftpFileReader.cs

@@ -1,4 +1,6 @@
-using Renci.SshNet.Abstractions;
+//#define READAHEAD_DISPOSE
+
+using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
 using System;
 using System.Collections.Generic;
@@ -8,6 +10,11 @@ namespace Renci.SshNet.Sftp
 {
     internal class SftpFileReader : ISftpFileReader
     {
+#if READAHEAD_DISPOSE
+        private static readonly TimeSpan ReadAheadCompleteTimeout = TimeSpan.FromSeconds(5);
+        private static readonly TimeSpan ReadAheadSemaphoreWaitTimeout = TimeSpan.FromMilliseconds(1000);
+#endif // READAHEAD_DISPOSE
+
         private readonly byte[] _handle;
         private readonly ISftpSession _sftpSession;
         private readonly uint _chunkSize;
@@ -83,6 +90,9 @@ namespace Renci.SshNet.Sftp
                 if (_exception != null)
                     throw _exception;
 
+                //if (_disposed)
+                //    throw new ObjectDisposedException(GetType().FullName);
+
                 if (nextChunk.Offset == _offset)
                 {
                     var data = nextChunk.Data;
@@ -168,23 +178,31 @@ namespace Renci.SshNet.Sftp
 
         protected void Dispose(bool disposing)
         {
+            if (_disposed)
+                return;
+
             if (disposing)
             {
+                _disposed = true;
+
+#if READAHEAD_DISPOSE
                 var readAheadCompleted = _readAheadCompleted;
                 if (readAheadCompleted != null)
                 {
-                    if (!readAheadCompleted.WaitOne(TimeSpan.FromSeconds(1)))
+                    if (!readAheadCompleted.WaitOne(ReadAheadCompleteTimeout))
                     {
-                        DiagnosticAbstraction.Log("Read-ahead thread did not complete within time-out.");
+                        throw new Exception();
+                        //DiagnosticAbstraction.Log("Read-ahead thread did not complete within time-out.");
                     }
                     readAheadCompleted.Dispose();
                     _readAheadCompleted = null;
                 }
+#endif // READAHEAD_DISPOSE
 
                 _sftpSession.RequestClose(_handle);
-
-                _disposed = true;
             }
+
+            _disposed = true;
         }
 
         private void StartReadAhead()
@@ -197,7 +215,22 @@ namespace Renci.SshNet.Sftp
                     // TODO implement cancellation!?
                     // TODO implement IDisposable to cancel the Wait in case the client never completes reading to EOF
                     // TODO check if the BCL Semaphore unblocks wait on dispose (and mimick same behavior in our SemaphoreLight ?)
+#if READAHEAD_DISPOSE
+                    if (!_semaphore.Wait(ReadAheadSemaphoreWaitTimeout))
+                    {
+                        if (_disposed)
+                        {
+                            lock (_readLock)
+                            {
+                                Monitor.Pulse(_readLock);
+                            }
+                            break;
+                        }
+                        continue;
+                    }
+#else
                     _semaphore.Wait();
+#endif // READAHEAD_DISPOSE
 
                     // don't bother reading any more chunks if we received EOF, or an exception has occurred
                     // while processing a chunk

+ 0 - 6
src/Renci.SshNet/Sftp/SftpFileStream.cs

@@ -188,14 +188,9 @@ namespace Renci.SshNet.Sftp
 
             // Initialize the object state.
             _session = session;
-            _bufferPosition = 0;
-            _bufferLen = 0;
-            _bufferOwnedByWrite = false;
             _canRead = ((access & FileAccess.Read) != 0);
             _canSeek = true;
             _canWrite = ((access & FileAccess.Write) != 0);
-            _position = 0;
-            _serverFilePosition = 0;
 
             var flags = Flags.None;
 
@@ -258,7 +253,6 @@ namespace Renci.SshNet.Sftp
 
             if (mode == FileMode.Append)
             {
-                _attributes = _session.RequestFStat(_handle, false);
                 _position = _attributes.Size;
                 _serverFilePosition = (ulong) _attributes.Size;
             }

+ 3 - 22
src/Renci.SshNet/Sftp/SftpSession.cs

@@ -130,28 +130,9 @@ namespace Renci.SshNet.Sftp
             return string.Format(CultureInfo.InvariantCulture, "{0}{1}{2}", canonizedPath, slash, pathParts[pathParts.Length - 1]);
         }
 
-        public ISftpFileReader CreateFileReader(string fileName, uint bufferSize)
+        public ISftpFileReader CreateFileReader(byte[] handle, ISftpSession sftpSession, uint chunkSize, int maxPendingReads, long? fileSize)
         {
-            var handle = RequestOpen(fileName, Flags.Read);
-
-            long? fileSize;
-            int maxPendingReads;
-
-            var chunkSize = CalculateOptimalReadLength(bufferSize);
-
-            var fileAttributes = RequestFStat(handle, true);
-            if (fileAttributes == null)
-            {
-                fileSize = null;
-                maxPendingReads = 5;
-            }
-            else
-            {
-                fileSize = fileAttributes.Size;
-                maxPendingReads = Math.Min(10, (int)Math.Ceiling((double)fileAttributes.Size / chunkSize) + 1);
-            }
-
-            return _serviceFactory.CreateSftpFileReader(handle, this, chunkSize, maxPendingReads, fileSize);
+            return new SftpFileReader(handle, sftpSession, chunkSize, maxPendingReads, fileSize);
         }
 
         internal string GetFullRemotePath(string path)
@@ -922,7 +903,7 @@ namespace Renci.SshNet.Sftp
         /// <returns>
         /// File attributes
         /// </returns>
-        internal SftpFileAttributes RequestStat(string path, bool nullOnError = false)
+        public SftpFileAttributes RequestStat(string path, bool nullOnError = false)
         {
             SshException exception = null;
 

+ 3 - 3
src/Renci.SshNet/SftpClient.cs

@@ -537,7 +537,7 @@ namespace Renci.SshNet
 
             var fullPath = _sftpSession.GetCanonicalPath(path);
 
-            var attributes = _sftpSession.RequestLStat(fullPath);
+            var attributes = _sftpSession.RequestStat(fullPath);
 
             return new SftpFile(_sftpSession, fullPath, attributes);
         }
@@ -1498,7 +1498,7 @@ namespace Renci.SshNet
         /// Sets the date and time the specified file was last accessed.
         /// </summary>
         /// <param name="path">The file for which to set the access date and time information.</param>
-        /// <param name="lastAccessTime">A <see cref="System.DateTime"/> containing the value to set for the last access date and time of path. This value is expressed in local time.</param>
+        /// <param name="lastAccessTime">A <see cref="DateTime"/> containing the value to set for the last access date and time of path. This value is expressed in local time.</param>
         [Obsolete("Note: This method currently throws NotImplementedException because it has not yet been implemented.")]
         public void SetLastAccessTime(string path, DateTime lastAccessTime)
         {
@@ -1995,7 +1995,7 @@ namespace Renci.SshNet
 
             var fullPath = _sftpSession.GetCanonicalPath(path);
 
-            using (var fileReader = _sftpSession.CreateFileReader(fullPath, _bufferSize))
+            using (var fileReader = ServiceFactory.CreateSftpFileReader(fullPath, _sftpSession, _bufferSize))
             {
                 var totalBytesRead = 0UL;