subsonic / SubSonic-3.0

SubSonic 3.0 for the .NET 3.5 Framework
http://subsonic.github.io/
558 stars 210 forks source link

DbDataReader must be closed in SubSonicRepository<T>.Add(T item, IDataProvider provider) #176

Open FabienDehopre opened 14 years ago

FabienDehopre commented 14 years ago

The DbDataReader used in SubSonicRepository.Add(T item, IDataProvider provider) for retrieving the last identity value must be closed before returning to avoid exception when using a SharedDbConnectionScope.

The solution can be to use a "using" statement so the DbDataReader will be dispose (closed).

mkoenings commented 13 years ago

I am using subsonic3 with mysql on linux (mono 2.6) and had serious problems because on every "add" a mysql session was left open. The session (session pooling) couldnt be closed, because the GC couldnt release it, because of the open reader! (my simple assumptions...) I put a rdr.close() in the Add method: that fixed the problem (sessions are getting released now) There might be a better way of doing this.

in SubSonicRepository.cs

public object Add(T item, IDataProvider provider) { var query = item.ToInsertQuery(provider).GetCommand(); object result = null; if(query != null) { if (provider.Client == DataClient.SqlClient) { //add in SCOPE_INDENTITY so we can pull back the ID query.CommandSql += "; SELECT SCOPE_IDENTITY() as new_id"; }

            var rdr = provider.ExecuteReader(query);
            if (rdr.Read())
            //TODO: fix in release
            //BUG: reader not closed properly
                result = rdr[0];
           // here comes the hack:
            rdr.Close();
          // end hack

            // repopulate primary key column with newly generated ID
            if (result != null && result != DBNull.Value) {

                try {
                    var tbl = provider.FindOrCreateTable(typeof(T));
                    var prop = item.GetType().GetProperty(tbl.PrimaryKey.Name);
                    var settable = result.ChangeTypeTo(prop.PropertyType);
                    prop.SetValue(item, settable, null);

                } catch (Exception x) {

                    //swallow it - I don't like this per se but this is a convenience and we
                    //don't want to throw the whole thing just because we can't auto-set the value
                }
            }

        }
        return result;
    }

See my awesome Mono/Subsonic site: www.birneninfo.de :-)