0

I have a WCF web service that accesses a WCF windows service on a different machine. The windows service does all the data accessing and passes the results to the web service. I have read several articles about disposing the service client for a WCF service correctly, but I'm not sure what the best way is to do this in a web service. (If this helps, the web service is PerCall not PerSession)

This is all I'm doing right now:

 Public Class Service1
  Implements IService1

  Private p_oWindowsService As DataService.Service1Client

  Public Sub New()
    p_oWindowsService = New DataService.Service1Client
  End Sub

  Public Function GetData(ByVal value As Integer) As String Implements IService1.GetData
    Return p_oWindowsService.GetData(value)
  End Function

  Public Function GetDataUsingDataContract(ByVal composite As CompositeType) As CompositeType Implements IService1.GetDataUsingDataContract
    If composite Is Nothing Then
      Throw New ArgumentNullException("composite")
    End If
    If composite.BoolValue Then
      composite.StringValue &= "Suffix"
    End If
    Return composite
  End Function

I'm not disposing the service client at all right now, from what I've read this is a major issue. The workaround I'm looking at is something like this inside the GetData function:

 Public Function GetData(ByVal value As Integer) As String Implements IService1.GetData
     Using oWindowsService As New DataService.Service1Client
        Return oWindowsService.GetData(value)
     End Using        
  End Function

Based off What is the best workaround for the WCF client `using` block issue?, I know I shouldn't actually depend on the using block. But should I be creating and disposing a service client in every function? That's my real question.

Thank you.

Community
  • 1
  • 1
cjw
  • 344
  • 1
  • 8
  • 23

2 Answers2

2

Yes do not use dispose. Do it like this:

var client = new ...;
try {
   // Do work

   // Everything went well so close the client
   client.Close();
}
catch( Exception ex ) {
   // Something went wrong so call abort
   client.Abort();

   // Other logging code 
}

if( client.State != System.ServiceModel.CommunicationState.Closed ) {
   client.Close();
}

Calling Close() on client notifies the service instance that it is no longer in use and may be collected by GC (subject to service instance management).

You may wonder why Abort in the catch block? The reason is:

Given the WCF binding use transport sessions, the client after a fault would not even be able to close it (if there was no transport layer session then the client could use or close the proxy, but this is not recommended as the configuration of sessions could change). So after a fault has happened the only safe operation is to abort a proxy.

See this for more on Abort vs Close.


EDIT

In your comments you asked:

Do you recommend creating and closing a service client like this within every function the web service calls the windows service?

No I don't think that is necessary. Let's see you are calling the WCF service from a web application (ASP MVC), then you would do something like this in the Dispose method of the controller since ClientBase<TChannel> implements ICommunicationObject:

protected override void Dispose(bool disposing) {
     base.Dispose( disposing );

     ServiceHelper.DisposeService( ( this._whateverServiceClient as ICommunicationObject ) );
 }

And here is the ServiceHelper class that you can use from anywhere:

public static class ServiceHelper {

  /// <summary>
  /// Disposes the given service.
  /// </summary>
  /// <param name="service">The service.</param>
  public static void DisposeService(ICommunicationObject service) {

     if( service != null ) {

        bool abort = true;

        try {
           if( service.State == CommunicationState.Opened || service.State == CommunicationState.Opening ) {
              service.Close();
              abort = false;
           }
        }
        finally {

           // Determine if we need to Abort the communication object.
           if( abort )
              service.Abort();
        }
     }
  }

}

The idea would be the same if you were calling it from another client.

Community
  • 1
  • 1
CodingYoshi
  • 25,467
  • 4
  • 62
  • 64
  • Do you recommend creating and closing a service client like this within every function the web service calls the windows service? – cjw Feb 13 '17 at 22:35
  • Thank you for this. I wanted this solution but use the generic Basechannel wich of course didn't work. ICommunicationObject was what I needed. This is the best solution so far. I made some adjustments though: public static void DisposeService(ICommunicationObject service) { if (service != null) { try { service.Close(); } catch { service.Abort(); } } } – Tomas Hesse Apr 30 '19 at 13:13
0

You don't need to explicitly dispose the client, how ever if you truly must, this is one way of properly closing & disposing your client:

        // use client
        try
        {
            ((IClientChannel)client).Close();
        }
        catch
        {
            ((IClientChannel)client).Abort();
        }
        finally
        {
            ((IDisposable)client).Dispose();
        }
Jorge Del Conde
  • 265
  • 1
  • 4
  • Calling Dispose() in a finally block is asking for trouble. Dispose() calls Close(), which can throw an exception if the channel is faulted. – tom redfern Feb 14 '17 at 09:20
  • The code sample that I provided is safe to use and will not throw. If Close() throws, Abort() will be called which will set the internal state to 'Closed'. On calling Dispose(), Close() will be called again with a state of Closed, which will do nothing. Like I said: You don't need to explicitly dispose the client but if you truly must, the above code shows a safe way of doing so. – Jorge Del Conde Feb 14 '17 at 16:15
  • Close() will return immediately if the ICommunicationObject is in the Closing state. Therefore, it is still possible for the ICommunicationObject to become faulted, at which point calling Dispose() will result in an exception. – tom redfern Feb 14 '17 at 16:46
  • No because as you already pointed out Dispose calls Close, and if Close returns immediately because of the State then no exception can occur. That's why Abort is being called if any type of exception occurs when Close is originally called. https://referencesource.microsoft.com/#System.ServiceModel/System/ServiceModel/Channels/ServiceChannel.cs,1796 BTW: Abort() provides the guarantee that no exceptions will be thrown. – Jorge Del Conde Feb 14 '17 at 20:20
  • You are assuming that if a channel is in the Closing state then it cannot become faulted. That may not be correct. – tom redfern Feb 14 '17 at 20:25
  • The only way I can think of that the channel may become faulted when Abort() is called is if one hooks up a poorly implemented event for either Closing or Closed and they retry to open the channel where this time it fails so it becomes faulted. If this is the case, then something better raise enough flags to tell the developer there's something terribly wrong in their code :) – Jorge Del Conde Feb 14 '17 at 20:43
  • Apologies if I've not been clear. I'm not talking about when Abort() is called. Your code block allows for the case where only the try...finally blocks are executed. So if the initial Close() call returns then the assumption is that it is safe to call Dispose(). Which I don't think is correct. – tom redfern Feb 14 '17 at 21:59
  • When Close() is called the state will be set to Closing and the original state captured. The original state will be either Created, Opening, Faulted, Opened, Closed, or Closing. If it's Created, Opening, or Faulted Abort() will be called and set the state to Closing/Closed. If the original state is Opened the state will become Closed. The next call to Close() will cause the flow to match Closing/Closed in the switch statement thus no-op. Unless one hooks up the OnClosing/OnClosed events and does something funky calling Dispose (while redundant) is perfectly fine and safe. – Jorge Del Conde Feb 14 '17 at 22:50