Disposen in ASP.NET

[Tarabass]

Ik heb een discussie gehad met mijn senior developer, waarin ik aangaf dat ik het raar vond dat in al bestaande code/objecten (gemaakt door een oud-collega) er niets gedisposed wordt. Ik heb het over unmanaged objects zoals sql-objecten etc.. Ik ben gewend deze altijd te disposen (using-statement), maar dit vond mijn senior totaal overbodig. Zijn onderbouwing was dat disposen in ASP.NET geen zin heeft omdat het stateless is en omdat de GC het wel zou opruimen. Daarnaast worden de connections met de SQL-server bewaakt door de application pool dus ik hoefde me nergens druk over te maken.

Maar toch bekruipt mij het gevoel dat dit onzin is. Is het echt onzin om objecten te disposen in ASP.NET? Worden unmanaged objects ook opgeruimd nadat de request is afgerond? En hoe kan ik onderbouwen (test-code, feiten) waarom ik gelijk heb?

Natuurlijk hoop ik dat mijn senior gelijk heeft

[mulder]

Mijn haren staan enigzins overeind

[Tarabass]

mulder schreef op dinsdag 05 oktober 2010 @ 21:24:
Mijn haren staan enigzins overeind

Leg uit

[mulder]

Van MSDN:

"The garbage collector in the CLR does not (and cannot) dispose of unmanaged objects, objects that the operating system executes directly, outside the CLR environment. This is because different unmanaged objects must be disposed of in different ways. That information is not directly associated with the unmanaged object; it must be found in the documentation for the object. A class that uses unmanaged objects must dispose of them in its Finalize method."

[Tarabass]

mulder schreef op dinsdag 05 oktober 2010 @ 21:43:
Van MSDN:

[...]

Dit heb ik inderdaad ook gelezen, net zoals vele blogs waarin disposen wordt aangeraden. Maar hoe heeft dit invloed op de performance van een groot portaal? ik zou het zelf altijd doen (disposen), maar hoe kan ik aantonen dat het beter is en ik toestemming krijg de objecten te verbouwen?

[yade]

Unmanaged objecten moet je altijd zelf opruimen, maar sql objecten, zijn die unmanaged?

Voor zaken als bijvoorbeeld SqlConnection objecten die de GC wel op zal ruimen is het tóch handig om deze zo snel mogelijk op te ruimen. Connection pooling zorgt er alleen maar voor dat database verbindingen met dure handshakes gewoon open blijven. Als je ervoor kiest om de SqlConnection niet te sluiten of te disposen dan zal er zodra er een nieuwe verbinding nodig is, een nieuwe verbinding worden gemaakt met als gevolg dat in een ongunstige situatie onnodig veel verbindingen naar de database worden gemaakt, en dat alleen maar omdat er nog verbindingen in gebruik zijn door SqlConnection objecten die wachten om eens een keertje opgeruimd te worden door de GC.

Om het kort samen te vatten: Pas als Close of Dispose wordt aangeroepen gaat de in gebruik zijnde verbinding terug naar de connectionpool.

Daarnaast is het gewoon good practice om een class dat IDisposable implementeert zelf op te ruimen.

Verder is ASP.NET niet stateless. In tegendeel.

[ Voor 16% gewijzigd door yade op 05-10-2010 22:17 ]

[Sebazzz]

mulder schreef op dinsdag 05 oktober 2010 @ 21:43:
Van MSDN:

[...]

Ik vind wel dat je moet disposen, maar lees ook wat er staat:

"The garbage collector in the CLR does not (and cannot) dispose of unmanaged objects, objects that the operating system executes directly, outside the CLR environment. This is because different unmanaged objects must be disposed of in different ways. That information is not directly associated with the unmanaged object; it must be found in the documentation for the object. A class that uses unmanaged objects must dispose of them in its Finalize method."

Indien er met "A class that ..." een IDisposable class bedoelen, betekent dus dat IDisposables hun Dispose methode in de Finalize methode moeten aanroepen of zeg ik nu iets doms?

[sig69]

Ja, want de finalizer (of destructor, wat je wil) wordt altijd aangeroepen als een object opgeruimd wordt. Je hebt echter geen enkele controle over wanneer dat gebeurd (kan sowieso wel even duren omdat objecten met een finalizer direct naar de Gen1 heap gaan), daarom vind ik het persoonlijk netter om wel Dispose aan te roepen, dan zijn de unmanaged resources in ieder geval alvast vrijgegeven.

[Tarabass]

Thx, mannen! Het is iets waar ik nooit echt over nadacht, good practice is waarom ik het altijd deed. Maar nu moet ik uitleggen waarom, en zeggen dat het het beste is voor je performance is niet genoeg.

En hoe kan ik dingen testen in code?

[yade]

Je zou iets kunnen doen met het openstaande connecties verhaal. Dit kan je zien in de database. Wees creatief.

[Tarabass]

yade schreef op dinsdag 05 oktober 2010 @ 22:30:
Je zou iets kunnen doen met het openstaande connecties verhaal. Dit kan je zien in de database. Wees creatief.

Ik ga er eens goed over nadenken. Als ik wat heb post ik het ook wel hier

[Razr]

Je zou een test kunnen schrijven waarbij je 1000x een for-lus uitvoert waarin een SqlConnection wordt geinstantieerd en gebruikt om een simpele query uit te voeren. Als je de connectie niet sluit en gebruikt maakt van pooling krijg je vanzelf een SqlException dat er geen connecties meer vanuit de pool verkregen kunnen worden.

Als je daaronder nog een for-lus schrijft maar dan met een using statement, of je sluit de connectie expliciet, dan zul je zien dat het daar wel goed gaat. Misschien een opzet voor een paar testen die je kunt aanvoeren als 'bewijs'?

[sig69]

Dat is in dat geval niet echt een goede test: de senior gaat er vanuit dat de garbage collector alles opruimt omdat alles stateless is. Je zou dus eigenlijk 1000 requests op een pagina die een simpele query uitvoert moeten afschieten.

[defcon84]

Tarabass schreef op dinsdag 05 oktober 2010 @ 21:19:
Ik heb een discussie gehad met mijn senior developer, waarin ik aangaf dat ik het raar vond dat in al bestaande code/objecten (gemaakt door een oud-collega) er niets gedisposed wordt. Ik heb het over unmanaged objects zoals sql-objecten etc.. Ik ben gewend deze altijd te disposen (using-statement), maar dit vond mijn senior totaal overbodig. Zijn onderbouwing was dat disposen in ASP.NET geen zin heeft omdat het stateless is en omdat de GC het wel zou opruimen. Daarnaast worden de connections met de SQL-server bewaakt door de application pool dus ik hoefde me nergens druk over te maken.

Maar toch bekruipt mij het gevoel dat dit onzin is. Is het echt onzin om objecten te disposen in ASP.NET? Worden unmanaged objects ook opgeruimd nadat de request is afgerond? En hoe kan ik onderbouwen (test-code, feiten) waarom ik gelijk heb?

Natuurlijk hoop ik dat mijn senior gelijk heeft

Dit is dus wat men een memory leak noemt.. en dus helemaal fout van je Senior

Sebazzz schreef op dinsdag 05 oktober 2010 @ 22:11:
[...]
Indien er met "A class that ..." een IDisposable class bedoelen, betekent dus dat IDisposables hun Dispose methode in de Finalize methode moeten aanroepen of zeg ik nu iets doms?

Met "A class" bedoelen ze hier gewoon 'een random class' en die hoeft zelf niet IDisposable te erven. Ze bedoelen dat er in die class objecten zijn die wel IDisposable erven.. en die moet je even manueel disposen als je je class gaat 'stoppen'..
bv:

C#:

public class MyClass
{
    System.IO.StreamReader reader;
    private bool isDisposed = false;

    // ctor
    public MyClass() {
        reader = new System.IO.StreamReader();
        isDisposed = false;
    }

    // disposer
    private void Dispose()
    {
        if (isDisposed) return;
        
        reader.Dispose();

        isDisposed = true;
    }

    // dtor
    ~MyClass()
    {
        Dispose();
    }
}

[Woy]

Het verhaal dat het stateless is, is sowieso onzin. Ja een HTTP request is stateless, maar de applicatie die in IIS draait zeer zeker niet. Objecten kunnen dus ook langer leven dan de lifetime van een request.

Het is gewoon good practice om voor alle objecten die IDisposable implementeren, en waar jij de owner van bent, Dispose aan te roepen op het moment dat het object end of life is. Op dat moment weet je zeker dat het object de mogelijkheid krijgt om zijn unmanaged resources vrij te geven. Als je dat niet doet betekend het niet zozeer dat je een memmory leak hebt, maar dat je resources voor een te lange periode claimt, en dat kan natuurlijk slecht zijn voor de performance.

Bij de Connections zit het echter nog een stukje complexer. Op het moment dat Dispose aangeroepen word, geeft de Connection zijn resources wel vrij, maar dat hoeft niet perse te betekenen dat de resources helemaal vrij gegeven worden, immers word hij gewoon terug gegeven aan de ConnectionPool. Maar ook dat is iets wat je zo snel mogelijk wil doen, want als je een Connection te lang vast houd, kan het wel zo zijn dat een ander moet wachten totdat hij een Connection krijgt toegewezen, of misschien word er wel een extra connection geopend, en dat kost natuurlijk ook weer resources.

defcon84 schreef op woensdag 06 oktober 2010 @ 09:10:
[...]
Dit is dus wat men een memory leak noemt..

Nee niet zozeer, het feit dat Dispose niet aangeroepen word, zegt niks over het feit of de resouce wel of niet vrij gegeven word, alleen wat over het moment waarop dat gebeurt.

en dus helemaal fout van je Senior

Dat inderdaad wel.

[ Voor 14% gewijzigd door Woy op 06-10-2010 09:17 ]

[Hydra]

Sebazzz schreef op dinsdag 05 oktober 2010 @ 22:11:
Indien er met "A class that ..." een IDisposable class bedoelen, betekent dus dat IDisposables hun Dispose methode in de Finalize methode moeten aanroepen of zeg ik nu iets doms?

Dat ze IDisposable implementeren betekent niet dat ze perse unmanaged resources gebruiken hoor, of dat het falikant misloopt als je ze niet disposed, maar het is m.i. "best practice" om ze netjes te disposen als je er klaar mee bent.

File handles worden bijvoorbeeld netjes vrijgegeven als een File object geGCed wordt, maar dat wil ook niet zeggen dat het niet netter is ze zelf te close()-en

Ik vind het verhaal van de "Senior" developer in de TS te triest voor woorden. Zoals iemand anders al aandroeg; een HTTP request mag dan in princiepe stateless zijn, maar je application is dat zeker niet. Unmanaged objecten worden gewoon pas opgeruimd als de applicatie zelf gestopt of gestart wordt.

[RobIII]

Hydra schreef op woensdag 06 oktober 2010 @ 09:41:
Dat ze IDisposable implementeren betekent niet dat ze perse unmanaged resources gebruiken hoor, of dat het falikant misloopt als je ze niet disposed, maar het is m.i. "best practice" om ze netjes te disposen als je er klaar mee bent.

Klopt en helemaal eensch, maar op het moment dat een class IDisposable implementeert en je er geen gebruik van maakt doe je aannames over de interne werking van de class. En dat kan inderdaad best goed gaan. Maar hoewel een class op dit moment misschien nog geen resources gebruikt kan dat in een later stadium/versie natuurlijk wel zo zijn. En daarom is het een best practice te disposen als een class IDisposable implementeert. Je hoort gewoon geen aannames te doen over wat een class intern doet. En assumptions are the mother of all fuckups Heerlijk als je die zo nu en dan kunt gebruiken.

[defcon84]

RobIII schreef op woensdag 06 oktober 2010 @ 10:10:
En assumptions are the mother of all fuckups

haha idd

komt er dus op neer dat je je eigen gewoonte moet aanhouden om Using gebruiken waar mogelijk..
en mss eens wisselen van titel met je college

[Woy]

defcon84 schreef op woensdag 06 oktober 2010 @ 10:55:
[...]
komt er dus op neer dat je je eigen gewoonte moet aanhouden om Using gebruiken waar mogelijk..

using waar mogelijk, of als je member variabelen hebt die IDisposable implementeren, zelf ook IDisposable implementeren en in je Dispose methode de Dispose van je members aanroepen.

[RobIII]

Woy schreef op woensdag 06 oktober 2010 @ 11:28:
[...]

using waar mogelijk, of als je member variabelen hebt die IDisposable implementeren, zelf ook IDisposable implementeren en in je Dispose methode de Dispose van je members aanroepen.

Dat, of een try / finally waarin je iig in de finally zorgt voor een dispose.

[Woy]

RobIII schreef op woensdag 06 oktober 2010 @ 12:14:
[...]

Dat, of een try / finally waarin je iig in de finally zorgt voor een dispose.

Ik kan niet echt een situatie bedenken waar ik try / finally zou verkiezen boven using.

[RobIII]

Woy schreef op woensdag 06 oktober 2010 @ 12:15:
[...]

Ik kan niet echt een situatie bedenken waar ik try / finally zou verkiezen boven using.

Feitelijk is het zelfs hetzelfde:

quote: http://msdn.microsoft.com/en-us/library/yh598w02.aspx

The using statement ensures that Dispose is called even if an exception occurs while you are calling methods on the object. You can achieve the same result by putting the object inside a try block and then calling Dispose in a finally block; in fact, this is how the using statement is translated by the compiler.

Ik meende alleen dat using een .Net 2.0 feature was en dat je dus <2.0 een try / finally zou moeten gebruiken; maar daar vergiste ik me in

[ Voor 3% gewijzigd door RobIII op 06-10-2010 12:29 ]

[Woy]

RobIII schreef op woensdag 06 oktober 2010 @ 12:28:
[...]
Feitelijk is het zelfs hetzelfde:

Dat snap ik, maar using geeft veel beter aan wat het doel is, en is minder verbose

[RobIII]

Oh, maar ik ben het ook met je eensch dat je beter een using kunt gebruiken hoor

[Tarabass]

Bedankt voor de antwoorden, mannen. Hier kan ik wat mee. Het bevestigd mijn gedachten, en ga daar dus niet vanaf wijken. Ik heb mijn senior geconfronteerd met het feit dat ik er van overtuigd ben dat asp.net niet stateless is, waarop ik als antwoord kreeg dat het er aan ligt wat je onder stateless verstaat. Tja. Wel stond hij open voor het overtuigd worden van disposen (eigenlijk al raar dat dat nodig is) dus dat ga ik dan maar proberen. Verder blijf ik gewoon using gebruiken en alles disposen waarvan ik vind dat het gedisposed moet worden. Toch stom dat een senior je zo aan het twijfelen kan zetten, soms moet je toch jezelf volgen. Hopelijk krijg ik de kans om de objecten in de applicatie om te mogen bouwen

[defcon84]

jah.. everybody lies veel succes ermee