difi / oppslagstjeneste-klient-dotnet

.Net klient for integrasjon mot Oppslagstjenesten for kontakt og reservasjonregisteret
https://samarbeid.difi.no/kontakt-og-reservasjonsregisteret
6 stars 6 forks source link

De synkrone metodene på API-et går i deadlock når de kalles fra en ASP.NET-applikasjon #68

Closed andreasnilsen closed 8 years ago

andreasnilsen commented 8 years ago

Jeg skal ikke påstå å ha full oversikt over problematikken her, men så vidt jeg har forstått oppfører konsollapplikasjoner og ASP.NET/WPF-applikasjoner seg forskjellig når det gjelder bruk av tråder.

Ved bruk av de synkrone metodene som eksponeres via Difi.Oppslagstjeneste.Klient.OppslagstjenesteKlient (denne bør forresten få et interface) via konsollapplikasjon og Unit-tester (xUnit 2) har jeg ingen problemer, men når jeg tar i bruk samme kode via en ASP.NET webapplikasjon (Web Api 2), går all bruk av de synkrone metodene på denne klassen i deadlock.

Jeg ser at de synkrone metodene er implementert slik at de delegerer kallet videre til de asynkrone metodene, og kaller .Result direkte på tasken disse returnerer, f.eks.:

public IEnumerable<Person> HentPersoner(string[] personidentifikator, params Informasjonsbehov[] informasjonsbehov)
        {
            try
            {
                return HentPersonerAsynkront(personidentifikator, informasjonsbehov).Result;

Som igjen gjør:

        public async Task<IEnumerable<Person>> HentPersonerAsynkront(string[] personidentifikator, params Informasjonsbehov[] informasjonsbehov)
        {
            var requestEnvelope = new PersonsEnvelope(OppslagstjenesteKonfigurasjon.Avsendersertifikat, OppslagstjenesteKonfigurasjon.SendPåVegneAv, personidentifikator, informasjonsbehov);
            Log.Debug($"HentPersonerAsynkront(personidentifikator:{personidentifikator} , informasjonsbehov:{informasjonsbehov})");
            if (RequestAndResponseLog.IsDebugEnabled && OppslagstjenesteKonfigurasjon.LoggForespørselOgRespons)
            {
                RequestAndResponseLog.Debug(requestEnvelope.XmlDocument.OuterXml);
            }
            var responseDocument = await GetClient().SendAsync(requestEnvelope);
            if (RequestAndResponseLog.IsDebugEnabled && OppslagstjenesteKonfigurasjon.LoggForespørselOgRespons)
            {
                RequestAndResponseLog.Debug(responseDocument.Envelope.InnerXml);
            }
            var dtoObject = ValidateAndConvertToDtoObject<HentPersonerRespons>(requestEnvelope, responseDocument);
            var domainObject = DtoConverter.ToDomainObject(dtoObject);
            return domainObject.Personer;
        }

I følge diverse blogposter ( http://blog.stephencleary.com/2012/07/dont-block-on-async-code.html , http://enterprisecraftsmanship.com/2014/12/20/pitfalls-of-async-await/ , https://blog.ciber.no/2014/05/19/using-task-configureawaitfalse-to-prevent-deadlocks-in-async-code/ ) vil følgende skje (basert på min bristende tolkning av hva som skjer) :

  1. Kallet til .Result vil starte async-tasken, og blokke Request-tråden som venter på svaret fra .Result
  2. Ikke-async kode i HentPersonerAsynkront vil forsøkes utført i Request-tråden, som er blokkert

http://enterprisecraftsmanship.com/2014/12/20/pitfalls-of-async-await/ nevner:

If you are developing a third-party library, it is always vital to configure await in such a way that the rest of the method will be executed by an arbitrary thread from the thread pool.

Ved å pakke inn kallet til HentPersoner i en Task.Run<<IEnumerable<Person>>(() => _klient.HentPersoner(...)).Result kommer jeg midlertidig rundt problemet.

Hvis det er riktig at dette er en svakhet i klienten, vet jeg ikke hva beste løsning ev. er, men nevnte blogposter beskriver bl.a. at å kalle .ConfigureAwait(false) på tasken er en mulighet.

Hvis dette er en feil, eksisterer den antakelig også i SDP-klienten Difi.SikkerDigitalPost.Klient.Api.SikkerDigitalPostKlient , som ser ut til å bruke samme mekanisme for å eksponere synkrone metoder.

asjafjell commented 8 years ago

Først vil jeg bare takke deg for å ha tatt deg god tid til å lage en skikkelig problembeskrivelse. Du har til og med skissert en mulig løsning og gitt gode referanser.

Det du skisserer her er absolutt et reelt problem ved bruk av GUI-applikasjoner som implementerer klientbiblioteket. Ved å kalle klientbiblioteket synkront så vil det låse, nettopp fordi vi kaller .Result på tasken. Vi må nok grave litt dypere og teste litt mer før vi legger til ConfigureAwait(false), men det ser ut som det løser problemet med deadlock i noen tilfeller.

Hvorfor bruker ikke du async-overloadene for å unngå låsing?

andreasnilsen commented 8 years ago

Integrasjonen er skrevet under tidspress, så jeg ønsket i utgangspunktet ikke å ta inn kompleksiteten av å måtte ta stilling til async, dvs. ved å la det boble helt opp til toppen av applikasjonen (web api-controllerne i denne sammenhengen).

Men jeg ser at det antakelig er den mest robuste løsningen, workaround jeg skisserte (spawne ny task) ser ikke ut til å være stabil.

For øvrig takk til Difi for at dere lager disse klientbibliotekene, og legger det ut på Github/Nuget! Veldig mye enklere og mer smidig enn f.eks. DSF-integrasjon.

kristianenge commented 8 years ago

Hei @andreasnilsen ,

Vi har testet litt med en veldig tynn Web-app lokalt og det ser ut til å fungere med .configureAwait(false). Kunne du verifisert at det fungerer i applikasjonen din også? 5.3.6023.25101-beta

andreasnilsen commented 8 years ago

Hei!

Jeg har skrevet om aktuell løsning til å benytte de asynkrone operasjonene, men skal se om jeg kan få verifisert det i løpet av helgen.

andreasnilsen commented 8 years ago

Jeg rekker ikke å få testet dette før ferien, da jeg ev. må endre gjennomgående i hele applikasjonsstacken for å skrive om "tilbake til" synkront kall, men jeg ser at #70 er verifisert OK med feilrettingen fra annen bruker, så da kan kanskje denne lukkes også :) ?