0

I have an ASP.NET application, which uses a 'service reference' to a 3rd-party/offsite payment processor. The sample code which I downloaded from the payment processor includes the following:

   public class SoapAPIUtilities{

      private static CustomerProfileWS.Service service = null;

      public static CustomerProfileWS.Service Service{
         get{
            if(service == null){
               service = new CustomerProfileWS.Service();
            }
            return service;
         }
      }   
   }

I generate CustomerProfileWS.Service automatically using Visual Web Developer: its auto-generated implementation is a subclass of subclass of ServiceModel.ClientBase, which MSDN documents as, "Any instance members are not guaranteed to be thread safe".

To use this service from ASP.NET pages, I guess I need to make access to the service thread-safe, which the above is not?

If it is not thread-safe then what is the better way to make it thread-safe:

  • Wrap an accessor class which implements a lock around the static singleton (e.g. as shown here)?
  • Don't use a static singleton; instead create multiple/temporary CustomerProfileWS.Service instances as needed, as local variables in methods of the ASP.NET pages which need them?
Community
  • 1
  • 1
ChrisW
  • 54,973
  • 13
  • 116
  • 224

3 Answers3

1

Making a proxy thread safe and singleton is not suitable for web applications.

The proxy channel could become faulted while another thread is using it. Also, you don't want to lock the channel for every operation because a website has many requests at the same time and they will be processed in row, waiting for a long running task, like calling a webservice.

You need to create one new proxy channel for each ASP .NET request. This is an expensive operation, so you might want to look at this article to make sure that the parts that are expensive to create are cached:

Performance Improvement for WCF Client Proxy Creation

Another way is to create a Pool of proxies, so that you don't have to create a new proxy each time, but reuse an existing one if it is available. An example can be found here:

A Sample for WCF Client Proxy Pooling

Cosmin Vană
  • 1,562
  • 12
  • 28
  • +1 Thank you for the references. [My example of a wrapper around a singleton](http://codereview.stackexchange.com/q/38953/34757) tries to recover from a fault, by recreating the singleton if it has faulted. – ChrisW Jan 13 '14 at 20:11
  • That won't work in this situation: Thread1 makes 1 call. Thread2 makes another call. Thread1 call fails and the channel becomes faulted. Thread2 cannot continue the operation because the channel is already faulted by Thread1. Even if you recover the proxy, two of your web requests (or as many as you have at the same time) will fail because one of them had a problem. – Cosmin Vană Jan 13 '14 at 20:15
  • I think it will work? Threads "make a call" by instantiating the Authorize class in a using statement e.g. `using (Authorize auth = new Authorize()) { auth->IsAlive(); }` There's a 'lock' (`Monitor.Enter`) which ensure that Thread2's call to the `Authorize` constructor won't complete until after Thread1 has disposed its Authorize instance, leaving the singleton in either a good state or a failed state. If it is failed, then Thread2's Authorize constructor recreates the singleton, after it acquires the lock, before it returns to the caller and let the caller invoke other Authorize messages. – ChrisW Jan 13 '14 at 20:42
  • If the proxy channel is locked for 0.1 seconds and you have 100 user requests to that page at the same time, the channel will be locked for 10 seconds for the last request. Depending how you use the lock, the code might fail if it will not be able to access a resource in 10 seconds. You also lose scalability when we talk about users (you could have 1000 users accessing pages that require that proxy channel). – Cosmin Vană Jan 13 '14 at 20:50
0

I would go with the 2nd option you suggest...

Don't use a static singleton; instead create multiple/temporary CustomerProfileWS.Service instances as needed, as local variables in methods of the ASP.NET pages which need them?

xspydr
  • 3,030
  • 3
  • 31
  • 49
  • Please tell me your reasoning? My worry about this method is that a) I don't know whether it's necessary b) I can't test whether the remote/server end correctly handles concurrent requests from the same client/merchant/site c) Because that's not how they do it in their 'sample program' it may not be supported. – ChrisW Jan 10 '14 at 15:19
  • By recreating your CustomerProfileWS.Service object for each payment being processed you completely eliminate a multi-thread scenario. For that matter I would highly discourage spinning up multiple threads in an ASP.NET application to begin with. – xspydr Jan 10 '14 at 15:24
  • I'm not spinning up threads. But if there's more than one concurrent user of the web site, then isn't there more than one ASPX page instance (each with its own thread) at the same time? – ChrisW Jan 10 '14 at 15:27
  • There is more than one instance as far as IIS is concerned. However, as far as your code is concerned only one request at a time would be accessing that code block if you are not spinning up new threads yourself... – xspydr Jan 10 '14 at 15:31
  • If there are two web pages running at the same time, then they might both try to access the same channel instance, if the channel is a static singleton as shown in the sample (not because I'm spinning up threads but because ASP.NET page instances can execute concurrently). – ChrisW Jan 10 '14 at 15:34
  • Your WCF service though is hosted in IIS, correct? If that's the case then your ASP.NET requests to the WCF service will never cross paths. If you were writing a client/desktop app whereby your WCF service was being accessed through a socket then the story would be different... – xspydr Jan 10 '14 at 15:37
  • It's not my WCF service: I'm a client, the server-side is an offsite payment processor called [authorize.net](https://api.authorize.net/soap/v1/Service.asmx). I hope its server-side is well-implemented, but I can't prove/test that: so perhaps serializing my accesses to it would be safer. What I'm asking about (what my ASP.NET code is using) is the client-side stub of the WCF channel. – ChrisW Jan 10 '14 at 15:42
  • Each ASP.NET request will execute the code block independently of each other. Each client will call authorize.net via their own request. This will not result in a cross-threaded situation from your applications perspective. As for authorize.net. They should have a way for you to send test payments through so that you can write unit tests to properly test your code. – xspydr Jan 10 '14 at 16:13
  • I see what you're saying now: they will be independent, if (but only if) each page has its own channel instance. I can send test payments to authorize.net but I can't guarantee whether my multiple concurrent requests arrive there simultaneously. IMO testing can't prove whether an implementation is thread-safe (it can only, if you're lucky, prove that it's not thread-safe): so instead of by testing, IMO thread-safety needs to be proven by design or by code inspection. – ChrisW Jan 10 '14 at 16:30
  • True. A unit test is only going to give so much in this situation. – xspydr Jan 10 '14 at 16:36
0

I'd change it to private static readonly CustomerProfileWS.Service service = new CustomerProfileWS.Service();.

This will create your singleton in a thread safe way.

However, if multiple threads access the WCF proxy, one could close the connection while the other is mid operation.

I think a proxy per ASP.NET request might be better suited here.

MattC
  • 3,984
  • 1
  • 33
  • 49
  • My code doesn't need to explicitly open or close the channel, does it? Assuming that my code never explicitly open or closes the channel, is the channel thread-safe? Does it implicitly serialize the requests, if it receives multiple concurrent requests? – ChrisW Jan 10 '14 at 17:51
  • Possibly look at http://stackoverflow.com/questions/3246634/do-i-need-to-close-a-net-service-reference-client-when-im-done-using-it – MattC Jan 13 '14 at 09:06