Hi,

I run the following code using the CLRProfiler.exe from Microsoft:

            DataAccessAdapter adapter = new DataAccessAdapter();
            try
            {
                MyEntity e = new MyEntity(myId);
                using (adapter)
                {
                    adapter.FetchEntity(e);
                }
                //Do something with entity e
            }
            catch (...)  { //Handle exception }

Both the CLRProfiler and the sample code is based on .Net Framework v2.0.50215.

After running the code I check the "Histogram" for "Objects finalized" (that is, which objects has been finalized by the Garbage Collector instead of disposed by the code running) and then I see that Oracle.DataAccess.Client.OracleCommand and Oracle.DataAccess.Client.OracleParameter is in the list of finalized objects. I would think that the command- and parameterobjects also should be disposed, but that seem not to be the case.

Letting the GC handle the cleanup will cause more use of resources then neccessary, especially since we're running this code in a server environment.

Is this a problem related to LLBLGen, and if so, is it possible to fix in a future release?

Using calls Dispose on DataAccessAdapter, and Dispose does:

/// <summary>
/// Implements the IDispose' method Dispose.
/// </summary>
public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}


/// <summary>
/// Implements the Dispose functionality. If a transaction is in progress, it will rollback that transaction.
/// </summary>
/// <param name="isDisposing">Flag which signals this routine if a dispose action should take place (true) or not (false)</param>
protected virtual void Dispose(bool isDisposing)
{
    // Check to see if Dispose has already been called.
    if(!_isDisposed)
    {
        if(isDisposing)
        {
            // Dispose managed resources.
            if(_physicalTransaction != null)
            {
                if(_isTransactionInProgress)
                {
                    Rollback();
                }
                else
                {
#if !CF
                    _physicalTransaction.Dispose();
#endif
                    _physicalTransaction = null;
                }
            }
            if(_activeConnection != null)
            {
                // closing the connection will abort (rollback) any pending transactions
                if(_activeConnection.State == ConnectionState.Open)
                {
                    _activeConnection.Close();
                }
#if !CF
                _activeConnection.Dispose();
#endif
                _activeConnection = null;
            }
        }
    }
    _isDisposed = true;
}
#endregion

what more can I do? Besides, oracleparameters and oraclecommand are managed objects, they don't need an extra dispose call, as oracle's connection dispose call (which is called, se code above) should take care of that if necessary.

Letting the GC handle the cleanup will cause more use of resources then neccessary, especially since we're running this code in a server environment.

GC always handles the cleanup. Disposing the connection will just release unmanaged resources, which is performed.

Is this a problem related to LLBLGen, and if so, is it possible to fix in a future release?

I don't see a problem, so why should there be a fix?

Never forget: .NET is a managed environment, with a GC which ONLY performs undeterministic cleanups. You can't nor should you want to control WHEN the GC cleans up objects.

AND: every object that goes out of scope doesn't have to be disposed. A disposed dataaccessadapter disposes the connection and transaction, which is the end of all objects related to it. Dispose of dataaccessadapter doesn't know which commands are active at that moment, nor does calling Dispose in a catch handler in the command execution logic help, nor is it necessary.

(edit): decompiling OracleConnection.Dispose, it frees (to me, but their naming is very cryptic) the data allocated by OracleCommand, as it should be. (compared to for example sqlclient: SqlConnection.Dispose cleans up everything, SqlCommand doesn't even implement IDisposable, nor overrides dispose()) Why they still implement IDisposable on OracleCommand is unclear to me, no docs available on that as well.

Hi

I need to respond to your post. IMHO it contains some inaccuracies that need to be corrected.

Besides, oracleparameters and oraclecommand are managed objects, they don't need an extra dispose call, as oracle's connection dispose call (which is called, se code above) should take care of that if necessary.

Any object that implements a finalizer will require more resources if it is not Disposed. If the GC needs to run the finalizer the object will be held in memory longer than otherwise nesseccary. This means that the object will hold on to its unmanaged resources longer than nesseccary.

Rico Mariani (One of the performance gurus at Microsoft) says the following: "When the garbage collector first encounters an object that is otherwise dead but still needs to be finalized it must abandon its attempt to reclaim the space for that object at that time. The object is instead added to a list of objects needing finalization and, furthermore, the collector must then ensure that all of the pointers within the object remain valid until finalization is complete. This is basically the same thing as saying that every object in need of finalization is like a temporary root object from the collector's perspective."

You can read more at: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndotnet/html/dotnetGCbasics.asp

So, I think that this actualy is a problem that requires a fix. I have looked at the source for LLBLGen and at a log from the ClrProfiler and found the following.

OracleCommand: From the allocation graph (and the source) I found that the OracleCommand instance was created by SD.LLBLGen.Pro.DQE.Oracle.DynamicQueryEngine.CreateSelectDQ. It is assigned to a RetrievalQuery and the query is returned to DataAccessAdapter.CreateSelectDQ. This in turn returns the query to DataAccessAdapterBase.FetchEntityUsingFilter where the query is used. The problem is that the query holds a reference to a disposable object, but does not implement IDisposable itself. This means that FetchEntityUsingFilter cannot dispose the OracleCommand in a sensible way. And since our code never sees this object we cannot correct this.

OracleParameter: This object is created by OracleSpecificCreator.CreateParameter and returned to FieldCompareValuePredicate.ToQueryText as IDataParameter. The problem here is that ToQueryText does not call dispose on any of the parameters that it created.

I hope this helps and that you can look at this issue again.

Regards, Trygve Valøy

(rephrased)

TValoy wrote:

Hi I need to respond to your post. IMHO it contains some inaccuracies that need to be corrected.

Besides, oracleparameters and oraclecommand are managed objects, they don't need an extra dispose call, as oracle's connection dispose call (which is called, se code above) should take care of that if necessary.

Any object that implements a finalizer will require more resources if it is not Disposed. If the GC needs to run the finalizer the object will be held in memory longer than otherwise nesseccary. This means that the object will hold on to its unmanaged resources longer than nesseccary.

IF the object holds unmanaged resources. And even then, they will get cleaned up.

Rico Mariani (One of the performance gurus at Microsoft) says the following: "When the garbage collector first encounters an object that is otherwise dead but still needs to be finalized it must abandon its attempt to reclaim the space for that object at that time. The object is instead added to a list of objects needing finalization and, furthermore, the collector must then ensure that all of the pointers within the object remain valid until finalization is complete. This is basically the same thing as saying that every object in need of finalization is like a temporary root object from the collector's perspective."

You can read more at: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndotnet/html/dotnetGCbasics.asp

So, I think that this actualy is a problem that requires a fix.

It's not a problem per se. Firstly, Oracle implemented their resource management in a very crappy way: instead of doing it like MS did on SqlClient, they spread all resources across a couple of objects. For example, if you dispose an OracleCommand, the OracleParameter isn't disposed, while a parameter can't be associated with multiple commands.

Secondly, because Oracle decided to implement IDisposable on almost all their objects, doesn't mean I should call IDisposable.Dispose(bool) on all these objects. First of all because it's generic code and not all ADO.NET providers implement IDisposable on a lot of objects (for example SqlParameter doesn't implement IDisposable, OracleParameter does). This thus requires reflection if a method is there, which is slow. Second of all, a parameter is created at place X (in the predicate classes for example) but not executed there. It's executed / used somewhere else.

So I'm not sure if there is anything to gain really in the area of performance, in this situation, plus it can be cumbersome to actually call Dispose on commands, see below.

I have looked at the source for LLBLGen and at a log from the ClrProfiler and found the following.

OracleCommand: From the allocation graph (and the source) I found that the OracleCommand instance was created by SD.LLBLGen.Pro.DQE.Oracle.DynamicQueryEngine.CreateSelectDQ. It is assigned to a RetrievalQuery

or ActionQuery

and the query is returned to DataAccessAdapter.CreateSelectDQ. This in turn returns the query to DataAccessAdapterBase.FetchEntityUsingFilter where the query is used.

Or various other methods, depends on what you're doing.

The problem is that the query holds a reference to a disposable object, but does not implement IDisposable itself. This means that FetchEntityUsingFilter cannot dispose the OracleCommand in a sensible way. And since our code never sees this object we cannot correct this.

Methods never implement IDisposable, DataAccessAdapter does, it cleans up everything when Dispose is called, at least on the connection department, which holds all the resources on the server.

I can't call Dispose on a command either, because of a transaction. Say you start a transaction and then call a couple of methods on DataAccessAdapter. I'm unsure what will happen when I call dispose right after the command has been executed, as Dispose() can do anything on the command, probably clean up stuff that's till needed (in a COM+ transaction for example).

For example, Dispose on OracleCommand closes an open reader, which is for example also an active action query (which is also executed using a datareader internally by most ado.net providers), so what will happen when that's closed and you're still in a transaction? Undefined: some providers may do this, others that, and it can be unclear what will happen if a Dispose is called during a transaction on some provider...

OracleParameter: This object is created by OracleSpecificCreator.CreateParameter and returned to FieldCompareValuePredicate.ToQueryText as IDataParameter. The problem here is that ToQueryText does not call dispose on any of the parameters that it created.

True, because it's not used there, it's used somewhere else. Besides that, disposing a parameter will be slow because not every parameter in every ado.net provider implements IDisposable, so reflection has to be used to determine if the type implements IDisposable, which has a performance hit as well.

Let me ask you a question in return: do you run in memory problems, or did you just profile the code and wondered why dispose wasn't called?

I appreciate your concern, but I think you should relax a little. The thing with Dispose is that Microsoft's CLR implementation is a bit obscure in this: they require the developer to implement code which calls Dispose() however it's completely against the pattern they use otherwise of non-deterministic garbage collection. (GC.Collect() is also not deterministic)

Thanky you for a thorough answer. To answer you question: We do not have a memory problem... yet, but in trying to follow performance best-practices we have done some profiling of our code. What happends when we start running our application under heavy load is so far an open question.

I accept your arguments regarding the fact that you need your code to work with different providers and the undetermined consequences of doing this in transaction contexts. (The fact that Oracle's implementation isn't exactly to notch is not news to me.)

Since this is not a critical issue for us now, I am not going to badger you any more with this I will try to relax, but performance is not the area where relaxation comes easies. (or is most appreciated by my boss)

TValoy wrote:

Thanky you for a thorough answer. To answer you question: We do not have a memory problem... yet, but in trying to follow performance best-practices we have done some profiling of our code. What happends when we start running our application under heavy load is so far an open question.

I understand. Please be sure that IF you run into problems and it's related to this, I will add code for you to fix it. Though it will then take serious testing which can take a while.

I accept your arguments regarding the fact that you need your code to work with different providers and the undetermined consequences of doing this in transaction contexts. (The fact that Oracle's implementation isn't exactly to notch is not news to me.)

Since this is not a critical issue for us now, I am not going to badger you any more with this I will try to relax, but performance is not the area where relaxation comes easies. (or is most appreciated by my boss)

I understand, and if it leads to problems, let me know. This code is in production in very large oracle based applications (2000 tables or more, terabytes of data) and never had complaints about memory leaks or other slowness. I hope that helps a bit Thanks for your concerns and thoughts on the subject

To re-open this old thread, a fix is now in test, which calls Dispose() on commands and parameters (if applicable). This should solve this ODP.NET memory leak once and for all. We still believe this is an ODP.NET problem, but because Oracle is even more stubborn to fix errors than MS, we proceeded with adding code for this, as we found a way to add it without sacrificing performance as we first thought.

This is fixed in the next build of the 1.0.2005.1 runtime libraries.