THRIFT-2032 C# client leaks sockets/handles

Patch: Jens Geyer

diff --git a/compiler/cpp/src/generate/t_csharp_generator.cc b/compiler/cpp/src/generate/t_csharp_generator.cc
index 37800a5..3f432da 100644 (file)
--- 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<t_function*> functions = tservice->get_functions();

diff --git a/lib/csharp/src/Protocol/TProtocol.cs b/lib/csharp/src/Protocol/TProtocol.cs
index ea3762c..70920d4 100644 (file)
--- 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 1ad3bd8..eefa4f9 100644 (file)
--- 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 8ce1085..113a472 100644 (file)
--- 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)
             {