From: Roger Meier Date: Wed, 11 Apr 2012 21:21:41 +0000 (+0000) Subject: THRIFT-1412 Thrift Transport classes should manage the lifetime of objects implementi... X-Git-Tag: 0.9.1~415 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=b1ec4ccca167a7ee21689fb5dd7238310ac91758;p=common%2Fthrift.git THRIFT-1412 Thrift Transport classes should manage the lifetime of objects implementing IDisposable by implementing IDisposable themselves Patch: Joshua Garvin git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1325013 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/csharp/src/Server/TSimpleServer.cs b/lib/csharp/src/Server/TSimpleServer.cs index c6fd99ea..1099fc1d 100644 --- a/lib/csharp/src/Server/TSimpleServer.cs +++ b/lib/csharp/src/Server/TSimpleServer.cs @@ -95,39 +95,35 @@ namespace Thrift.Server TProtocol outputProtocol = null; try { - client = serverTransport.Accept(); - if (client != null) - { - inputTransport = inputTransportFactory.GetTransport(client); - outputTransport = outputTransportFactory.GetTransport(client); - inputProtocol = inputProtocolFactory.GetProtocol(inputTransport); - outputProtocol = outputProtocolFactory.GetProtocol(outputTransport); - while (processor.Process(inputProtocol, outputProtocol)) { } - } - } - catch (TTransportException ttx) - { - // Client died, just move on - if (stop) - { - logDelegate("TSimpleServer was shutting down, caught " + ttx.GetType().Name); - } - } - catch (Exception x) - { - logDelegate(x.ToString()); - } - - if (inputTransport != null) - { - inputTransport.Close(); - } - - if (outputTransport != null) - { - outputTransport.Close(); - } - } + using(client = serverTransport.Accept()) + { + if (client != null) + { + using(inputTransport = inputTransportFactory.GetTransport(client)) + { + using (outputTransport = outputTransportFactory.GetTransport(client)) + { + inputProtocol = inputProtocolFactory.GetProtocol(inputTransport); + outputProtocol = outputProtocolFactory.GetProtocol(outputTransport); + while (processor.Process(inputProtocol, outputProtocol)) { } + } + } + } + } + } + catch (TTransportException ttx) + { + // Client died, just move on + if (stop) + { + logDelegate("TSimpleServer was shutting down, caught " + ttx.GetType().Name); + } + } + catch (Exception x) + { + logDelegate(x.ToString()); + } + } if (stop) { diff --git a/lib/csharp/src/Server/TThreadPoolServer.cs b/lib/csharp/src/Server/TThreadPoolServer.cs index cf8354e9..b257fd8f 100644 --- a/lib/csharp/src/Server/TThreadPoolServer.cs +++ b/lib/csharp/src/Server/TThreadPoolServer.cs @@ -75,16 +75,20 @@ namespace Thrift.Server :base(processor, serverTransport, inputTransportFactory, outputTransportFactory, inputProtocolFactory, outputProtocolFactory, logDel) { - if (!ThreadPool.SetMinThreads(minThreadPoolThreads, minThreadPoolThreads)) - { - throw new Exception("Error: could not SetMinThreads in ThreadPool"); - } - if (!ThreadPool.SetMaxThreads(maxThreadPoolThreads, maxThreadPoolThreads)) - { - throw new Exception("Error: could not SetMaxThreads in ThreadPool"); + lock (typeof(TThreadPoolServer)) + { + if (!ThreadPool.SetMinThreads(minThreadPoolThreads, minThreadPoolThreads)) + { + throw new Exception("Error: could not SetMinThreads in ThreadPool"); + } + if (!ThreadPool.SetMaxThreads(maxThreadPoolThreads, maxThreadPoolThreads)) + { + throw new Exception("Error: could not SetMaxThreads in ThreadPool"); + } } } + /// /// Use new ThreadPool thread for each new client connection /// diff --git a/lib/csharp/src/Server/TThreadedServer.cs b/lib/csharp/src/Server/TThreadedServer.cs index f2be073b..8e73bb76 100644 --- a/lib/csharp/src/Server/TThreadedServer.cs +++ b/lib/csharp/src/Server/TThreadedServer.cs @@ -185,14 +185,18 @@ namespace Thrift.Server TProtocol outputProtocol = null; try { - inputTransport = inputTransportFactory.GetTransport(client); - outputTransport = outputTransportFactory.GetTransport(client); - inputProtocol = inputProtocolFactory.GetProtocol(inputTransport); - outputProtocol = outputProtocolFactory.GetProtocol(outputTransport); - while (processor.Process(inputProtocol, outputProtocol)) - { - //keep processing requests until client disconnects - } + using (inputTransport = inputTransportFactory.GetTransport(client)) + { + using (outputTransport = outputTransportFactory.GetTransport(client)) + { + inputProtocol = inputProtocolFactory.GetProtocol(inputTransport); + outputProtocol = outputProtocolFactory.GetProtocol(outputTransport); + while (processor.Process(inputProtocol, outputProtocol)) + { + //keep processing requests until client disconnects + } + } + } } catch (TTransportException) { @@ -202,15 +206,6 @@ namespace Thrift.Server logDelegate("Error: " + x); } - if (inputTransport != null) - { - inputTransport.Close(); - } - if (outputTransport != null) - { - outputTransport.Close(); - } - lock (clientLock) { clientThreads.Remove(Thread.CurrentThread); diff --git a/lib/csharp/src/Transport/TBufferedTransport.cs b/lib/csharp/src/Transport/TBufferedTransport.cs index 28a855a5..14b5db06 100644 --- a/lib/csharp/src/Transport/TBufferedTransport.cs +++ b/lib/csharp/src/Transport/TBufferedTransport.cs @@ -22,7 +22,7 @@ using System.IO; namespace Thrift.Transport { - public class TBufferedTransport : TTransport + public class TBufferedTransport : TTransport, IDisposable { private BufferedStream inputBuffer; private BufferedStream outputBuffer; @@ -96,5 +96,25 @@ namespace Thrift.Transport { outputBuffer.Flush(); } - } + + #region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + protected override void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (inputBuffer != null) + inputBuffer.Dispose(); + if (outputBuffer != null) + outputBuffer.Dispose(); + } + } + _IsDisposed = true; + } + #endregion + } } diff --git a/lib/csharp/src/Transport/TFramedTransport.cs b/lib/csharp/src/Transport/TFramedTransport.cs index e259f5a5..3d431120 100644 --- a/lib/csharp/src/Transport/TFramedTransport.cs +++ b/lib/csharp/src/Transport/TFramedTransport.cs @@ -16,12 +16,12 @@ * specific language governing permissions and limitations * under the License. */ - +using System; using System.IO; namespace Thrift.Transport { - public class TFramedTransport : TTransport + public class TFramedTransport : TTransport, IDisposable { protected TTransport transport = null; protected MemoryStream writeBuffer; @@ -93,6 +93,7 @@ namespace Thrift.Transport ((i32rd[2] & 0xff) << 8) | ((i32rd[3] & 0xff)); + byte[] buff = new byte[size]; transport.ReadAll(buff, 0, size); readBuffer = new MemoryStream(buff); @@ -133,5 +134,22 @@ namespace Thrift.Transport // Reserve space for message header to be put right before sending it out writeBuffer.Write ( header_dummy, 0, header_size ); } - } + #region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + protected override void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (readBuffer != null) + readBuffer.Dispose(); + } + } + _IsDisposed = true; + } + #endregion + } } diff --git a/lib/csharp/src/Transport/THttpClient.cs b/lib/csharp/src/Transport/THttpClient.cs index 717907c5..106f840d 100644 --- a/lib/csharp/src/Transport/THttpClient.cs +++ b/lib/csharp/src/Transport/THttpClient.cs @@ -27,7 +27,7 @@ using System.Threading; namespace Thrift.Transport { - public class THttpClient : TTransport + public class THttpClient : TTransport, IDisposable { private readonly Uri uri; private Stream inputStream; @@ -142,9 +142,11 @@ namespace Thrift.Transport byte[] data = outputStream.ToArray(); connection.ContentLength = data.Length; - Stream requestStream = connection.GetRequestStream(); - requestStream.Write(data, 0, data.Length); - inputStream = connection.GetResponse().GetResponseStream(); + using (Stream requestStream = connection.GetRequestStream()) + { + requestStream.Write(data, 0, data.Length); + inputStream = connection.GetResponse().GetResponseStream(); + } } catch (IOException iox) { @@ -156,7 +158,7 @@ namespace Thrift.Transport } } #endif - private HttpWebRequest CreateRequest() + private HttpWebRequest CreateRequest() { HttpWebRequest connection = (HttpWebRequest)WebRequest.Create(uri); @@ -356,5 +358,24 @@ namespace Thrift.Transport } #endif - } +#region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + protected override void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (inputStream != null) + inputStream.Dispose(); + if (outputStream != null) + outputStream.Dispose(); + } + } + _IsDisposed = true; + } +#endregion + } } diff --git a/lib/csharp/src/Transport/TServerSocket.cs b/lib/csharp/src/Transport/TServerSocket.cs index fd5c662c..1ad3bd89 100644 --- a/lib/csharp/src/Transport/TServerSocket.cs +++ b/lib/csharp/src/Transport/TServerSocket.cs @@ -126,22 +126,36 @@ namespace Thrift.Transport { TcpClient result = server.AcceptTcpClient(); TSocket result2 = new TSocket(result); - result2.Timeout = clientTimeout; - if (useBufferedSockets) - { - TBufferedTransport result3 = new TBufferedTransport(result2); - return result3; - } - else - { - return result2; - } - } - catch (Exception ex) - { - throw new TTransportException(ex.ToString()); - } - } + try + { + result2 = new TSocket(result); + result2.Timeout = clientTimeout; + if (useBufferedSockets) + { + TBufferedTransport result3 = new TBufferedTransport(result2); + return result3; + } + else + { + return result2; + } + } + catch (System.Exception) + { + // If a TSocket was successfully created, then let + // it do proper cleanup of the TcpClient object. + if (result2 != null) + result2.Dispose(); + else // Otherwise, clean it up ourselves. + ((IDisposable)result).Dispose(); + throw; + } + } + catch (Exception ex) + { + throw new TTransportException(ex.ToString()); + } + } public override void Close() { diff --git a/lib/csharp/src/Transport/TSocket.cs b/lib/csharp/src/Transport/TSocket.cs index feb5503c..c05b6c26 100644 --- a/lib/csharp/src/Transport/TSocket.cs +++ b/lib/csharp/src/Transport/TSocket.cs @@ -145,5 +145,24 @@ namespace Thrift.Transport client = null; } } - } + + #region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + protected override void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (client != null) + ((IDisposable)client).Dispose(); + base.Dispose(disposing); + } + } + _IsDisposed = true; + } + #endregion + } } diff --git a/lib/csharp/src/Transport/TStreamTransport.cs b/lib/csharp/src/Transport/TStreamTransport.cs index 60a8412a..901b6096 100644 --- a/lib/csharp/src/Transport/TStreamTransport.cs +++ b/lib/csharp/src/Transport/TStreamTransport.cs @@ -103,5 +103,26 @@ namespace Thrift.Transport outputStream.Flush(); } - } + + + #region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + protected override void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (InputStream != null) + InputStream.Dispose(); + if (OutputStream != null) + OutputStream.Dispose(); + } + } + _IsDisposed = true; + } + #endregion + } } diff --git a/lib/csharp/src/Transport/TTransport.cs b/lib/csharp/src/Transport/TTransport.cs index 3fbc5d67..c03e9c22 100644 --- a/lib/csharp/src/Transport/TTransport.cs +++ b/lib/csharp/src/Transport/TTransport.cs @@ -25,7 +25,7 @@ using System; namespace Thrift.Transport { - public abstract class TTransport + public abstract class TTransport : IDisposable { public abstract bool IsOpen { @@ -82,5 +82,17 @@ namespace Thrift.Transport public virtual void EndFlush(IAsyncResult asyncResult) { } - } + + #region " IDisposable Support " + // IDisposable + protected abstract void Dispose(bool disposing); + + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(ByVal disposing As Boolean) above. + Dispose(true); + GC.SuppressFinalize(this); + } + #endregion + } }