views:

397

answers:

5

Hi,

I'm using Linq to SQL and read in a blog post about closing database connections as soon as possible. As an example, they showed a variable being converted to a list (using .ToList()) instead of actually returning the Linq query. I have the below code:

 public static bool HasPassword(string userId)
 {

    ProjDataContext db = new ProjDataContext();

    bool hasPassword = (from p in db.tblSpecUser
                                    where p.UserID == userId
                                    select p.HasPassword).FirstOrDefault();


    return hasPassword;
 }

Is that query fine? Or will the database connection remain open for longer than necessary?

Thank you for any advice

A: 

With Linq-To-SQL you generally don't need to be concerned about specifically opening and closing the connection that is part of the context object (db in your example). About the only time you would have to specifically do this is if you were sending direct SQL calls through the context object, instead of using Linq.

With L2S, you generally want to create your context object, do your unit of work, and then dispose of the object, as quickly as possible. Your code example looks fine to me.

Randy Minder
+2  A: 

The connection will be managed automatically. However, there are (or at least can be as the comments suggest) additional resouces associated with the DataContext. These resources will not be released until the DataContext is destroyed by the garbage collector. So, it is usually better to make sure that dispose is called when you don't need the DataContext anymore.

using (ProjDataContext db = new ProjDataContext()) {
    bool hasPassword = (from p in db.tblSpecUser
                                    where p.UserID == userId
                                    select p.HasPassword).FirstOrDefault();


    return hasPassword;
}

Here it is ensured that db.Dispose() is called when the using block exits, thus closing the connection explicitly.

Edit: Following the discussion I looked at the DataContext dispose myself (also using Reflector) and found the following code (FW 3.5) which gets called from DataContext.Dispose:

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        if (this.provider != null)
        {
            this.provider.Dispose();
            this.provider = null;
        }
        this.services = null;
        this.tables = null;
        this.loadOptions = null;
    }
}

So there are resources that gets freed:

  • The provider which may hold a DbConnection, a log (TextWriter) and a DbTransaction.
  • The the CommonDataServices.
  • The tables dictionary.
  • The LoadOptions.

The provider may hold resources that needs to be disposed (DbConnection and DbTransaction). Also the TextWriter for the log may have to be disposed, depending upon what instance of the TextWriter the user has assigned to the DataContext's logging mechanism, e.g. a FileWriter that then gets closed automatically.

The other properties hold, as far as I understand them -without looking too much into detail - only memory, but this is also made available for garbage collection by the dispose method, however, the it is not determined when the memory actually gets freed.

So, finally I totally agree with casparOne's statement:

In general, sharing data-access resources like this is a bad idea.

You should create your resources to access the DB, perform your operations, and then dispose of them when done.

Obalix
Wow, thanks for the quick replies! I'll replace all my projdatacontext code to use the 'using' method instead of creating a new instance and waiting for its Dispose method being called. Thanks again for the help.
o-logn
It's worth noting that, although IDisposable is implemented in Linq to SQL, the designers of Linq to SQL state specifically that the DataContext does **not** need to be disposed, most of the time. See http://stackoverflow.com/questions/821574/c-linq-to-sql-should-datacontext-be-disposed-using-idisposable/821595
Robert Harvey
Thanks for the information. If I understand correctly, does that mean that although 'using' is good to use when you need tighter control over the GC timing, it's not really required for when you're using DataContext (most of the time)?
o-logn
@o-logn: That is my understanding.
Robert Harvey
Thanks for the help, Robert.
o-logn
@Robert Harvey: Even though the author of a type says that Dispose doesn't need to be called, it doesn't make the type future-proof. It is coding against the *implementation* and not the *contract* which should *always* be avoided if it can be. It's bad practice not to.
casperOne
@casperOne: Microsoft has no plans to extend or otherwise improve on Linq to Sql anyway, so future compatibility is probably a moot point. Your statement is tantamount to saying that the designers made L2S implementation-specific, and therefore made an error in their design. You might be right. But are you then going to go against their advice on how to use the type just because you want to be *correct?* That only makes sense if there is no negative impact by doing so.
Robert Harvey
@Robert Harvey: Correct that there are no plans to improve on L2S, but in the event one wants to move to a different provider, say LINQ-to-Entities (which is a *very* real scenario), you have a massive refactoring project underway if you didn't code against the contract vs. the implementation. The general design patterns for LINQ providers (asside from LINQ-to-Objects) is to have a context that implements IDisposable, and again, it's always a best-practice to code against contract unless absolutely necessary to code against implementation.
casperOne
@casparOne and @Robert Harvey: Could you please provide sourcee (= references not code) for your statements that Linq to SQL does not do anything in the Dispose method. The reference posted suggests that you should call Dispose because there **can be** additional resouces that need to be freed. Also, what is about the cahed objects do they not get freed? The post sugests something different. Also most samples on MS **do** use using.
Obalix
`most samples on MS do use using` - Can you provide some examples? If most samples use `using`, I'm inclined to do it that way. I have yet to see one, except here on SO.
Robert Harvey
@casperOne: The Dispose method disposes the `provider` member, and sets the `services`, `tables`, and `loadOptions` members to null. The Dispose method in `SqlProvider` (called by the DataContext Dispose method) closes the `connection` member, and sets a bunch of other members to null. So I wouldn't exactly say that it does nothing.
Robert Harvey
@Obalix: I obtained my information on the Dispose method by running Reflector on it. http://www.red-gate.com/products/reflector/
Robert Harvey
@casparOne and @Rober Harvey: I follow the argumentation of casparOne, and edited the post accordingly.
Obalix
A: 

I think is a good practice to use the using statement. But I think there is nothing bad with your query.

public static bool HasPassword(string userId)
 {

    using(var db = new ProjDataContext())
    {

       bool hasPassword = (from p in db.tblSpecUser
                                    where p.UserID == userId
                                    select p.HasPassword).FirstOrDefault();


        return hasPassword;
    }
}
Jonathan
A: 

The database connection will be closed immediately after your db object will no longer exist (disposed) or explicitly closed. In your sample it will get (sooner or later) garbage collected.

Savvas Sopiadis
+1  A: 

From an implementation point of view, no, you don't have anything to worry about. However, it's not due to the query, but to the management of the DataContext itself.

The DataContext class implements the IDisposable interface, so you should call Dispose on the DataContext implementation whenever you are done with it.

Now, it's a well known fact that calling Dispose on DataContext instances do nothing, and therefore are not technically required.

Unfortunately, it's also very bad practice. You should always code against the contract, not the implementation. Because DataContext implements IDisposable, you should close it, even if you know it does nothing because that could absolutely change in future implementations.

Also, if you switch to another LINQ provider, say LINQ-to-Entities, then you must call Dispose when you are done, because the lifetime of database connections in ObjectContext instances (which also implements IDisposable) is very different, and a call to Dispose has an impact on those database connections.

All that being said, you have a bigger concern. If you are sharing one DataContext, you run the risk of tracking too many objects. Unless you have set the ObjectTrackingEnabled property to false, the DataContext is tracking every object selected through it. If you are doing no update operations (or even if you are) over the lifetime of the app, the number of resources being dedicated for object tracking on a shared DataContext could become considerable.

The rules developing using for other database technologies (e.g. the classes in the System.Data.SqlClient namespace) still apply.

In general, sharing data-access resources like this is a bad idea.

You should create your resources to access the DB, perform your operations, and then dispose of them when done.

casperOne
Thanks for the post. The code I posted is pretty much how I've implemented all the methods in my DAL. Am I still sharing DataContext's if I create a new instance per method as in the code?
o-logn
@o-logn: No, if you create a new instance when you need it (say, in a using statement, creating and disposing of it per operation) then that would be "coding against the contract" as mentioned above, and the right way to use DataContext instances, IMO.
casperOne
@casperOne - Ah, that's a relief. Thought I was in for a huge re-coding session :). Thanks!
o-logn