Skip to content

Commit

Permalink
Fixed a memory leak caused by holding references to custom entity map…
Browse files Browse the repository at this point in the history
…ping overrides
  • Loading branch information
MoonStorm committed Feb 3, 2016
1 parent 1f8d6f9 commit 9fa9cb9
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 107 deletions.
36 changes: 5 additions & 31 deletions Dapper.FastCRUD/EntityDescriptors/TypedEntityDescriptor.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
namespace Dapper.FastCrud.EntityDescriptors
{
using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Threading;
using Dapper.FastCrud.Mappings;
using Dapper.FastCrud.SqlBuilders;
Expand All @@ -13,12 +12,9 @@
/// </summary>
internal class EntityDescriptor<TEntity>:EntityDescriptor
{
private static readonly long _cleanupTimespan = TimeSpan.Zero.Ticks;
private readonly Stopwatch _lastCleanupTime;

// entity mappings should have a very long timespan if used correctly, however we can't make that assumption
// hence we'll cache them for the duration of their lifespan and clean them up periodically
private readonly ConcurrentDictionary<Guid, Tuple<WeakReference<EntityMapping>, ISqlStatements>> _registeredEntityMappings;
// hence we'll have to keep them for the duration of their lifespan and attach precomputed sql statements
private readonly ConditionalWeakTable<EntityMapping, ISqlStatements> _registeredEntityMappings;
private readonly Lazy<ISqlStatements> _defaultEntityMappingSqlStatements;

/// <summary>
Expand All @@ -28,10 +24,8 @@ public EntityDescriptor()
:base(typeof(TEntity))
{
this.DefaultEntityMapping = new AutoGeneratedEntityMapping<TEntity>();
_registeredEntityMappings = new ConcurrentDictionary<Guid, Tuple<WeakReference<EntityMapping>, ISqlStatements>>();
_defaultEntityMappingSqlStatements = new Lazy<ISqlStatements>(()=>this.ConstructSqlStatements(this.DefaultEntityMapping), LazyThreadSafetyMode.PublicationOnly);
_lastCleanupTime = new Stopwatch();
_lastCleanupTime.Start();
_registeredEntityMappings = new ConditionalWeakTable<EntityMapping, ISqlStatements>();
}

/// <summary>
Expand All @@ -47,14 +41,7 @@ public ISqlStatements GetSqlStatements(EntityMapping entityMapping = null)
}
else
{
var cachedStatements = _registeredEntityMappings.GetOrAdd(entityMapping.Id, mappingId => new Tuple<WeakReference<EntityMapping>, ISqlStatements>(new WeakReference<EntityMapping>(entityMapping), this.ConstructSqlStatements(entityMapping)));
sqlStatements = cachedStatements.Item2;
if (_lastCleanupTime.ElapsedTicks > _cleanupTimespan)
{
// we might hit this multiple times until we reset the counter, but that only means we'll perform the cleanup multiple times
_lastCleanupTime.Restart();
ThreadPool.UnsafeQueueUserWorkItem(state => this.CleanupOrphanEntityMappings(), null);
}
sqlStatements = _registeredEntityMappings.GetValue(entityMapping, mapping => this.ConstructSqlStatements(mapping));
}

return sqlStatements;
Expand Down Expand Up @@ -88,18 +75,5 @@ private ISqlStatements ConstructSqlStatements(EntityMapping entityMapping)
sqlStatements = new GenericSqlStatements<TEntity>(statementSqlBuilder);
return sqlStatements;
}

private void CleanupOrphanEntityMappings()
{
EntityMapping dummyEntityMapping;
Tuple<WeakReference<EntityMapping>, ISqlStatements> dummySqlStatements;
foreach (var cachedMappingStatements in _registeredEntityMappings)
{
//if (!cachedMappingStatements.Value.Item1.TryGetTarget(out dummyEntityMapping))
{
_registeredEntityMappings.TryRemove(cachedMappingStatements.Key, out dummySqlStatements);
}
}
}
}
}
71 changes: 1 addition & 70 deletions Dapper.FastCRUD/Mappings/EntityMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@
/// Holds information about table mapped properties for a particular entity type.
/// Multiple instances of such mappings can be active for a single entity type.
/// </summary>
public abstract class EntityMapping : IEquatable<EntityMapping>
public abstract class EntityMapping
{
private volatile bool _isFrozen;
private readonly IDictionary<string, PropertyMapping> _propertyMappings;
private readonly Guid _id;

/// <summary>
/// Default constructor.
/// </summary>
protected EntityMapping(Type entityType)
{
_propertyMappings = new Dictionary<string, PropertyMapping>();
_id = Guid.NewGuid();
this.EntityType = entityType;
this.TableName = entityType.Name;
this.Dialect = OrmConfiguration.DefaultDialect;
Expand Down Expand Up @@ -72,11 +70,6 @@ internal IDictionary<string, PropertyMapping> PropertyMappings
}
}

/// <summary>
/// Gets the id associated with the current instance.
/// </summary>
internal Guid Id => _id;

internal void FreezeMapping()
{

Expand Down Expand Up @@ -122,68 +115,6 @@ protected PropertyMapping SetPropertyInternal(PropertyMapping propertyMapping)
{
return this.PropertyMappings[propertyMapping.PropertyName] = propertyMapping;
}

/// <summary>
/// Indicates whether the current object is equal to another object of the same type.
/// </summary>
/// <returns>
/// true if the current object is equal to the <paramref name="other"/> parameter; otherwise, false.
/// </returns>
/// <param name="other">An object to compare with this object.</param>
public bool Equals(EntityMapping other)
{
if (ReferenceEquals(null, other))
{
return false;
}
if (ReferenceEquals(this, other))
{
return true;
}
return _id.Equals(other._id);
}

/// <summary>
/// Determines whether the specified object is equal to the current object.
/// </summary>
/// <returns>
/// true if the specified object is equal to the current object; otherwise, false.
/// </returns>
/// <param name="obj">The object to compare with the current object. </param>
public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
{
return false;
}
if (ReferenceEquals(this, obj))
{
return true;
}
var other = obj as EntityMapping;
return other != null && Equals(other);
}

/// <summary>
/// Serves as the default hash function.
/// </summary>
/// <returns>
/// A hash code for the current object.
/// </returns>
public override int GetHashCode()
{
return _id.GetHashCode();
}

public static bool operator ==(EntityMapping left, EntityMapping right)
{
return Equals(left, right);
}

public static bool operator !=(EntityMapping left, EntityMapping right)
{
return !Equals(left, right);
}
}

/// <summary>
Expand Down
14 changes: 8 additions & 6 deletions NuGetSpecs/Dapper.FastCRUD.nuspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,30 @@ For .NET 4.5, the package will also install the dependency 'StringInterpolationB

Features:
----------
- Support for LocalDb, Ms Sql Server, MySql, SqLite, PostgreSql
- Composite primary keys are supported
- Support for LocalDb, Ms Sql Server, MySql, SqLite, PostgreSql.
- Composite primary keys are supported.
- Compatible with most EF data annotations.
- Multiple entity mappings are supported, useful for partial queries in large denormalized tables and data migrations between different database types.
- All the CRUD methods accept a transaction, a command timeout, and a custom entity mapping.
- Fast pre-computed entity queries
- Fast pre-computed entity queries.
- A useful SQL builder and statement formatter which can be used even if you don't need the CRUD features of this library.
- A generic T4 template for C# is also provided for convenience in the NuGet package Dapper.FastCrud.ModelGenerator.
Code first entities are also supported which can either be decorated with attributes, have their mappings programmatically set, or using your own custom convention.

Please check the project site for usage and benchmarks.
Check out the package on nuget.org for release notes. Usage and benchmarks can be found on the project site.
</description>
<releaseNotes>
2.3.1:
2.3.2:
- Duplicate delimiters avoided if an already delimited identifier was passed as an alias.
- Fixed a memory leak caused by holding references to custom entity mapping overrides.

2.3.0:
- BREAKING CHANGE: the logic for properties marked with DatabaseGeneratedAttribute has changed.
- BREAKING CHANGE: the fluent way of specifying database generated values for the columns, including default values, has changed.
- BREAKING CHANGE: entities are being updated with the computed values on return from an UPDATE.
- NotMappedAttribute is now supported.
- DatabaseGeneratedDefaultValueAttribute was introduced to mark the properties bound to columns having default values that should be retrieved from the database.
- More information about these breaking changes can be found on the project site.
- More information about the breaking changes can be found on the project site.

2.2.x:
- BREAKING CHANGE: due to the large number of arguments, in some cases, a new definition was added for all the extension methods and the existing methods were marked as obsolete with some of them losing argument optionality.
Expand Down

0 comments on commit 9fa9cb9

Please sign in to comment.