From 102bca47f754d9c9ed6ce341c7f8f106bd2719d7 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Tue, 25 Jun 2013 22:21:29 +0200 Subject: [PATCH] THRIFT-2032 C# client leaks sockets/handles Patch: Jens Geyer --- .../cpp/src/generate/t_csharp_generator.cc | 31 ++++++++ lib/csharp/src/Protocol/TProtocol.cs | 25 +++++- lib/csharp/src/Transport/TServerSocket.cs | 78 +++++++++---------- tutorial/csharp/CsharpClient/CsharpClient.cs | 67 ++++++++-------- 4 files changed, 130 insertions(+), 71 deletions(-) diff --git a/compiler/cpp/src/generate/t_csharp_generator.cc b/compiler/cpp/src/generate/t_csharp_generator.cc index 37800a5c..3f432da9 100644 --- a/compiler/cpp/src/generate/t_csharp_generator.cc +++ b/compiler/cpp/src/generate/t_csharp_generator.cc @@ -1244,6 +1244,8 @@ void t_csharp_generator::generate_service_client(t_service* tservice) { if (tservice->get_extends() != NULL) { extends = type_name(tservice->get_extends()); extends_client = extends + ".Client, "; + } else { + extends_client = "IDisposable, "; } generate_csharp_doc(f_service_, tservice); @@ -1290,6 +1292,35 @@ void t_csharp_generator::generate_service_client(t_service* tservice) { indent(f_service_) << "get { return oprot_; }" << endl; scope_down(f_service_); f_service_ << endl << endl; + + indent(f_service_) << "#region \" IDisposable Support \"" << endl; + indent(f_service_) << "private bool _IsDisposed;" << endl << endl; + indent(f_service_) << "// IDisposable" << endl; + indent(f_service_) << "public void Dispose()" << endl; + scope_up(f_service_); + indent(f_service_) << "Dispose(true);" << endl; + scope_down(f_service_); + indent(f_service_) << endl << endl; + indent(f_service_) << "protected virtual void Dispose(bool disposing)" << endl; + scope_up(f_service_); + indent(f_service_) << "if (!_IsDisposed)" << endl; + scope_up(f_service_); + indent(f_service_) << "if (disposing)" << endl; + scope_up(f_service_); + indent(f_service_) << "if (iprot_ != null)" << endl; + scope_up(f_service_); + indent(f_service_) << "((IDisposable)iprot_).Dispose();" << endl; + scope_down(f_service_); + indent(f_service_) << "if (oprot_ != null)" << endl; + scope_up(f_service_); + indent(f_service_) << "((IDisposable)oprot_).Dispose();" << endl; + scope_down(f_service_); + scope_down(f_service_); + scope_down(f_service_); + indent(f_service_) << "_IsDisposed = true;" << endl; + scope_down(f_service_); + indent(f_service_) << "#endregion" << endl; + f_service_ << endl << endl; } vector functions = tservice->get_functions(); diff --git a/lib/csharp/src/Protocol/TProtocol.cs b/lib/csharp/src/Protocol/TProtocol.cs index ea3762c2..70920d47 100644 --- a/lib/csharp/src/Protocol/TProtocol.cs +++ b/lib/csharp/src/Protocol/TProtocol.cs @@ -27,7 +27,7 @@ using Thrift.Transport; namespace Thrift.Protocol { - public abstract class TProtocol + public abstract class TProtocol : IDisposable { protected TTransport trans; @@ -41,6 +41,29 @@ namespace Thrift.Protocol get { return trans; } } + #region " IDisposable Support " + private bool _IsDisposed; + + // IDisposable + public void Dispose() + { + Dispose(true); + } + + protected virtual void Dispose(bool disposing) + { + if (!_IsDisposed) + { + if (disposing) + { + if (trans is IDisposable) + (trans as IDisposable).Dispose(); + } + } + _IsDisposed = true; + } + #endregion + public abstract void WriteMessageBegin(TMessage message); public abstract void WriteMessageEnd(); public abstract void WriteStructBegin(TStruct struc); diff --git a/lib/csharp/src/Transport/TServerSocket.cs b/lib/csharp/src/Transport/TServerSocket.cs index 1ad3bd89..eefa4f97 100644 --- a/lib/csharp/src/Transport/TServerSocket.cs +++ b/lib/csharp/src/Transport/TServerSocket.cs @@ -116,48 +116,48 @@ namespace Thrift.Transport } } - protected override TTransport AcceptImpl() - { - if (server == null) - { - throw new TTransportException(TTransportException.ExceptionType.NotOpen, "No underlying server socket."); - } - try - { - TcpClient result = server.AcceptTcpClient(); - TSocket result2 = new TSocket(result); - try - { - result2 = new TSocket(result); - result2.Timeout = clientTimeout; - if (useBufferedSockets) - { - TBufferedTransport result3 = new TBufferedTransport(result2); - return result3; - } - else - { - return result2; - } - } - catch (System.Exception) + protected override TTransport AcceptImpl() { - // 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; + if (server == null) + { + throw new TTransportException(TTransportException.ExceptionType.NotOpen, "No underlying server socket."); + } + try + { + TSocket result2 = null; + TcpClient result = server.AcceptTcpClient(); + 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()); + } } - } - catch (Exception ex) - { - throw new TTransportException(ex.ToString()); - } - } - public override void Close() + public override void Close() { if (server != null) { diff --git a/tutorial/csharp/CsharpClient/CsharpClient.cs b/tutorial/csharp/CsharpClient/CsharpClient.cs index 8ce10852..113a4722 100644 --- a/tutorial/csharp/CsharpClient/CsharpClient.cs +++ b/tutorial/csharp/CsharpClient/CsharpClient.cs @@ -37,45 +37,50 @@ namespace CSharpTutorial Calculator.Client client = new Calculator.Client(protocol); transport.Open(); + try + { + client.ping(); + Console.WriteLine("ping()"); - client.ping(); - Console.WriteLine("ping()"); + int sum = client.add(1, 1); + Console.WriteLine("1+1={0}", sum); - int sum = client.add(1, 1); - Console.WriteLine("1+1={0}", sum); + Work work = new Work(); - Work work = new Work(); + work.Op = Operation.DIVIDE; + work.Num1 = 1; + work.Num2 = 0; + try + { + int quotient = client.calculate(1, work); + Console.WriteLine("Whoa we can divide by 0"); + } + catch (InvalidOperation io) + { + Console.WriteLine("Invalid operation: " + io.Why); + } - work.Op = Operation.DIVIDE; - work.Num1 = 1; - work.Num2 = 0; - try - { - int quotient = client.calculate(1, work); - Console.WriteLine("Whoa we can divide by 0"); - } - catch (InvalidOperation io) - { - Console.WriteLine("Invalid operation: " + io.Why); - } + work.Op = Operation.SUBTRACT; + work.Num1 = 15; + work.Num2 = 10; + try + { + int diff = client.calculate(1, work); + Console.WriteLine("15-10={0}", diff); + } + catch (InvalidOperation io) + { + Console.WriteLine("Invalid operation: " + io.Why); + } + + SharedStruct log = client.getStruct(1); + Console.WriteLine("Check log: {0}", log.Value); - work.Op = Operation.SUBTRACT; - work.Num1 = 15; - work.Num2 = 10; - try - { - int diff = client.calculate(1, work); - Console.WriteLine("15-10={0}", diff); } - catch (InvalidOperation io) + finally { - Console.WriteLine("Invalid operation: " + io.Why); + transport.Close(); } - - SharedStruct log = client.getStruct(1); - Console.WriteLine("Check log: {0}", log.Value); - - transport.Close(); } catch (TApplicationException x) { -- 2.17.1