Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark AddTypeHandlerImpl as obsolete and prevent lost updates via AddTypeHandler #2129

Merged
merged 7 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 44 additions & 26 deletions Dapper/SqlMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,14 @@ static SqlMapper()
[MemberNotNull(nameof(typeHandlers))]
private static void ResetTypeHandlers(bool clone)
{
typeHandlers = [];
AddTypeHandlerImpl(typeof(DataTable), new DataTableHandler(), clone);
AddTypeHandlerImpl(typeof(XmlDocument), new XmlDocumentHandler(), clone);
AddTypeHandlerImpl(typeof(XDocument), new XDocumentHandler(), clone);
AddTypeHandlerImpl(typeof(XElement), new XElementHandler(), clone);
lock (typeHandlersSyncLock)
{
typeHandlers = [];
AddTypeHandlerCore(typeof(DataTable), new DataTableHandler(), clone);
AddTypeHandlerCore(typeof(XmlDocument), new XmlDocumentHandler(), clone);
AddTypeHandlerCore(typeof(XDocument), new XDocumentHandler(), clone);
AddTypeHandlerCore(typeof(XElement), new XElementHandler(), clone);
}
}

/// <summary>
Expand Down Expand Up @@ -339,7 +342,7 @@ public static void RemoveTypeMap(Type type)
/// </summary>
/// <param name="type">The type to handle.</param>
/// <param name="handler">The handler to process the <paramref name="type"/>.</param>
public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerImpl(type, handler, true);
public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerCore(type, handler, true);
/// <summary>
/// Determine if the specified type will be processed by a custom handler.
/// </summary>
Expand All @@ -353,7 +356,16 @@ public static void RemoveTypeMap(Type type)
/// <param name="type">The type to handle.</param>
/// <param name="handler">The handler to process the <paramref name="type"/>.</param>
/// <param name="clone">Whether to clone the current type handler map.</param>
[Obsolete("Please use " + nameof(AddTypeHandler), error: true)]
[Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clone)
{
// this method was accidentally made public; we'll mark it as illegal, but
// preserve existing usage in compiled code; sorry about this!
AddTypeHandlerCore(type, handler, true); // do not allow suppress clone
}

private static void AddTypeHandlerCore(Type type, ITypeHandler? handler, bool clone)
{
if (type is null) throw new ArgumentNullException(nameof(type));

Expand All @@ -373,39 +385,45 @@ public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clo
}
}

var snapshot = typeHandlers;
if (snapshot.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do
// synchronize between callers mutating type-handlers; note that regular query
// code may still be accessing the field, so we still use snapshot/mutate/swap;
// the synchronize is just to prevent lost writes
lock (typeHandlersSyncLock)
{
if (typeHandlers.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do

var newCopy = clone ? new Dictionary<Type, ITypeHandler>(snapshot) : snapshot;
var newCopy = clone ? new Dictionary<Type, ITypeHandler>(typeHandlers) : typeHandlers;

#pragma warning disable 618
typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
if (secondary is not null)
{
typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
}
typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
if (secondary is not null)
{
typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
}
#pragma warning restore 618
if (handler is null)
{
newCopy.Remove(type);
if (secondary is not null) newCopy.Remove(secondary);
}
else
{
newCopy[type] = handler;
if (secondary is not null) newCopy[secondary] = handler;
if (handler is null)
{
newCopy.Remove(type);
if (secondary is not null) newCopy.Remove(secondary);
}
else
{
newCopy[type] = handler;
if (secondary is not null) newCopy[secondary] = handler;
}
typeHandlers = newCopy;
}
typeHandlers = newCopy;
}

/// <summary>
/// Configure the specified type to be processed by a custom handler.
/// </summary>
/// <typeparam name="T">The type to handle.</typeparam>
/// <param name="handler">The handler for the type <typeparamref name="T"/>.</param>
public static void AddTypeHandler<T>(TypeHandler<T> handler) => AddTypeHandlerImpl(typeof(T), handler, true);
public static void AddTypeHandler<T>(TypeHandler<T> handler) => AddTypeHandlerCore(typeof(T), handler, true);

private static Dictionary<Type, ITypeHandler> typeHandlers;
private static readonly object typeHandlersSyncLock = new();

internal const string LinqBinary = "System.Data.Linq.Binary";

Expand Down Expand Up @@ -479,7 +497,7 @@ public static void SetDbType(IDataParameter parameter, object value)
{
handler = (ITypeHandler)Activator.CreateInstance(
typeof(SqlDataRecordHandler<>).MakeGenericType(argTypes))!;
AddTypeHandlerImpl(type, handler, true);
AddTypeHandlerCore(type, handler, true);
return DbType.Object;
}
catch
Expand Down
4 changes: 2 additions & 2 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ skip_commits:
environment:
Appveyor: true
# Postgres
POSTGRES_PATH: C:\Program Files\PostgreSQL\13
POSTGRES_PATH: C:\Program Files\PostgreSQL\9.6
PGUSER: postgres
PGPASSWORD: Password12!
POSTGRES_ENV_POSTGRES_USER: postgres
Expand All @@ -32,7 +32,7 @@ environment:

services:
# - mysql
- postgresql13
- postgresql96

init:
- git config --global core.autocrlf input
Expand Down