ExecuteReader: is using statement enough?

Question

I'm using a web service, that is invoked every few ms, to expose some functionalities. Basically these functionalities are based on store procedures, and all my methods look like the follow:

[WebMethod]
public bool CheckMessageForMES( out int returnCode, out int messagePK, out String messageBody, out bool isRowFetched )
    {
        using ( SqlConnection connection = new SqlConnection() )
        {
            using(SqlCommand command = _dl.GetSqlCommandForStoredProcedure(DataLayer.SP_NAME, connection)){

            SqlParameter parameterReturnCode = _dl.CreateParameter("@returnCode", DbType.Int16, ParameterDirection.Output);
            SqlParameter parameterMessagePK = _dl.CreateParameter("@messagePK", DbType.Int32, ParameterDirection.Output);
            SqlParameter parameterMessageBody = _dl.CreateParameter("@messageBody", DbType.Xml, ParameterDirection.Output);
            SqlParameter parameterIsRowFetched = _dl.CreateParameter("@isRowFetched", DbType.Int16, ParameterDirection.Output);

            SqlParameter[] parameters = {
                                            parameterReturnCode,
                                            parameterMessagePK,
                                            parameterMessageBody,
                                            parameterIsRowFetched
                                        };

            command.Parameters.AddRange(parameters);

            connection.Open();

            using (SqlDataReader r = command.ExecuteReader() )
            {
                r.Close();
            }

            connection.Close();

            returnCode = int.Parse(parameterReturnCode.Value.ToString());
            messagePK = int.Parse(parameterMessagePK.Value.ToString());
            messageBody = parameterMessageBody.Value.ToString();
            isRowFetched = int.Parse(parameterIsRowFetched.Value.ToString()) > 0;
        }
 }

        return isRowFetched;
    }

Webserver process takes memory and never releases, and using VS10 tools the problem seems located in command.ExecuteReader(). Do you know why?

I'm implementing in the right way this method?

Thanks!

Don't call `GC.Collect` manually. It's almost never a good idea and will often result in higher memory usage in the short term. — Dan Puzey, Jul 31 '12 at 09:59
You should also wrap your SqlCommand in a using statement, see http://msdn.microsoft.com/en-us/library/bb348146%28v=vs.90%29.aspx. — Herman Cordes, Jul 31 '12 at 10:00
@DanPuzey: I put this call only after I noticed the huge memory usage. I can remove for sure, as the behaviour is the same... — ff8mania, Jul 31 '12 at 10:00
@Monty: Done. I'm going to update code to let you see the changes — ff8mania, Jul 31 '12 at 10:03
@achitaka-san: A method that creates and returns a SqlParameter — ff8mania, Jul 31 '12 at 10:27
No caching? Are you sure you are not keeping references to parameters inside `_dl`? For instance putting them in some fields, lists or dictionaries? If you do not release `SqlParameter` they will not be GC-ed causing leak. — George Mamaladze, Jul 31 '12 at 10:31
Method CreateParameter is: `internal SqlParameter CreateParameter( String parameterName, DbType type, ParameterDirection direction ) { SqlParameter parameter = new SqlParameter(parameterName, type); parameter.Direction = direction; parameter.DbType = type; return parameter; }` — ff8mania, Jul 31 '12 at 10:37
When you say "never releases," how long are you testing this for to confirm? A GC will only occur when the runtime deems it needed, and until then you may see higher memory usage. That doesn't mean that the memory *can't* be released, just that it doesn't *need* to be. How much memory is it hanging on to and how do you know it's *never* released? — Dan Puzey, Jul 31 '12 at 12:06
@DanPuzey I mean that an exception OutOfMemory is thrown, and webserver memory become bigger than 1.5gb...starting from few mb... — ff8mania, Jul 31 '12 at 12:17
@ff8mania: this is why *details* are important. An `OutOfMemoryException` does *not* mean you're leaking memory, it means that your application cannot *allocate* a required amount of memory. There are huge differences betwen these things! Rereading your question, I'm interested to know: you say you're calling every few *milliseconds* - how many milliseconds, exactly? Is your first call completing before the second call starts? — Dan Puzey, Jul 31 '12 at 13:00
@DanPuzey: Probably I was not precise, but what I meant is that due to this huge amount of memory used in time (the exception is raised after a couple of hours), the exception is raised as my application is not able to allocate further memory. Few milliseconds are 10. The calls are not parallel, so the second is called as soon as the first has done its job. — ff8mania, Jul 31 '12 at 15:14
I'm pretty sure your service call will take more than 10ms to complete. Are you sure you're not queueing up loads of service calls that are consuming memory? — Dan Puzey, Jul 31 '12 at 15:29
@DanPuzey May be, but I set a rate really fast because I was sure that queueing was not a problem. I cannot know before how much time my call will spend.... Anyway I'm not so sure that my call took more than 10 ms as it is a really simple stored procedure. Any further suggestion? — ff8mania, Aug 01 '12 at 07:10
Further suggestion? Turn your rate down, because I think your queueing is a problem. I am *absolutely* sure that the overhead of a webservice will take more than 10ms, even if your stored procedure does not, and if you end up with a dozen of these executing concurrently then it'll just grind to a halt. Run your service once a *second* instead: if you have a genuine leak, it'll still show up. — Dan Puzey, Aug 01 '12 at 07:22
@DanPuzey: Unfortunately I cannt turn my rate down, as I need to be very fast. Anyway I can put this rate to one second only to take a look to the result. If it is a leak, also with 1 second of rate the memory should be lost. Am I right? — ff8mania, Aug 01 '12 at 10:43
Correct; I was suggesting that you turn the rate down for testing purpose only. That said, you should definitely time your calls, because I very much doubt you're completing a call every 10ms. If you really require this time resolution, a web service may not be the best solution. — Dan Puzey, Aug 01 '12 at 10:54

score 1 · Answer 1 · answered Jul 31 '12 at 10:02

1

You never use the datarow you read, so you should use ExecuteNonQuery instead of the datareader.

answered Jul 31 '12 at 10:02

podiluska

50,950
7
98
104

Does that explain the memory leack? – George Mamaladze Jul 31 '12 at 10:07
If the leak is in executereader as asserted, eliminating it will logically get rid of the leak :) – podiluska Jul 31 '12 at 10:15
@podiluska : The question was about memory problem. Yes `ExecuteNonWuery` is better in this case, but there is nothing wrong in doing that the way it's done. Could you please explain why do you think this will fix the memory problem? – George Mamaladze Jul 31 '12 at 10:20
@ff8mania @achitaka-san: I don't think it would cause a leak, personally, but this is a fair answer since the question suggests the leak is in `ExecuteReader`. – Dan Puzey Aug 01 '12 at 07:19

ExecuteReader: is using statement enough?

1 Answers1