From af21e485e0ff810783ded46d4ebb20d1274120de Mon Sep 17 00:00:00 2001 From: Anairkoen Schno Date: Tue, 19 Jan 2021 16:09:50 -0600 Subject: [PATCH] Change transactions now properly only finish when all transactions exit, not just the first --- .../GeneratedStoreImpl/IGeneratedStore.cs | 24 +++++------ .../Stores/GeneratedStoreImpl/MakeCreator.cs | 43 +++---------------- 2 files changed, 18 insertions(+), 49 deletions(-) diff --git a/IPA.Loader/Config/Stores/GeneratedStoreImpl/IGeneratedStore.cs b/IPA.Loader/Config/Stores/GeneratedStoreImpl/IGeneratedStore.cs index 4b00daa0..350ef742 100644 --- a/IPA.Loader/Config/Stores/GeneratedStoreImpl/IGeneratedStore.cs +++ b/IPA.Loader/Config/Stores/GeneratedStoreImpl/IGeneratedStore.cs @@ -42,8 +42,7 @@ namespace IPA.Config.Stores internal class Impl : IConfigStore { private readonly IGeneratedStore generated; - private bool inChangeTransaction = false; - //private bool changedInTransaction = false; + private long enteredTransactions = 0; internal static ConstructorInfo Ctor = typeof(Impl).GetConstructor(new[] { typeof(IGeneratedStore) }); public Impl(IGeneratedStore store) => generated = store; @@ -104,7 +103,7 @@ namespace IPA.Config.Stores public static IDisposable ImplChangeTransaction(IGeneratedStore s, IDisposable nest) => FindImpl(s).ChangeTransaction(nest); // TODO: improve trasactionals so they don't always save in every case public IDisposable ChangeTransaction(IDisposable nest, bool takeWrite = true) - => GetFreeTransaction().InitWith(this, !inChangeTransaction, nest, takeWrite && !WriteSyncObject.IsWriteLockHeld); + => GetFreeTransaction().InitWith(this, nest, takeWrite && !WriteSyncObject.IsWriteLockHeld); private ChangeTransactionObj GetFreeTransaction() => freeTransactionObjs.Count > 0 ? freeTransactionObjs.Pop() @@ -117,23 +116,21 @@ namespace IPA.Config.Stores private struct Data { public readonly Impl impl; - public readonly bool owns; public readonly bool ownsWrite; public readonly IDisposable nested; - public Data(Impl impl, bool owning, bool takeWrite, IDisposable nest) + public Data(Impl impl, bool takeWrite, IDisposable nest) { - this.impl = impl; owns = owning; ownsWrite = takeWrite; nested = nest; + this.impl = impl; ownsWrite = takeWrite; nested = nest; } } private Data data; - public ChangeTransactionObj InitWith(Impl impl, bool owning, IDisposable nest, bool takeWrite) + public ChangeTransactionObj InitWith(Impl impl, IDisposable nest, bool takeWrite) { - data = new Data(impl, owning, takeWrite, nest); + data = new Data(impl, takeWrite, nest); - if (data.owns) - impl.inChangeTransaction = true; + _ = Interlocked.Increment(ref impl.enteredTransactions); if (data.ownsWrite) impl.TakeWrite(); @@ -143,9 +140,8 @@ namespace IPA.Config.Stores public void Dispose() => Dispose(true); private void Dispose(bool addToStore) { - if (data.owns) + if (data.impl != null && Interlocked.Decrement(ref data.impl.enteredTransactions) == 0) { - data.impl.inChangeTransaction = false; data.impl.InvokeChanged(); } data.nested?.Dispose(); @@ -157,6 +153,9 @@ namespace IPA.Config.Stores catch { } + + data = default; + if (addToStore) freeTransactionObjs.Push(this); } @@ -170,7 +169,6 @@ namespace IPA.Config.Stores return store?.Impl; } - internal static MethodInfo ImplReadFromMethod = typeof(Impl).GetMethod(nameof(ImplReadFrom)); public static void ImplReadFrom(IGeneratedStore s, ConfigProvider provider) => FindImpl(s).ReadFrom(provider); public void ReadFrom(ConfigProvider provider) diff --git a/IPA.Loader/Config/Stores/GeneratedStoreImpl/MakeCreator.cs b/IPA.Loader/Config/Stores/GeneratedStoreImpl/MakeCreator.cs index 58aa88a7..d9248c21 100644 --- a/IPA.Loader/Config/Stores/GeneratedStoreImpl/MakeCreator.cs +++ b/IPA.Loader/Config/Stores/GeneratedStoreImpl/MakeCreator.cs @@ -62,39 +62,6 @@ namespace IPA.Config.Stores var implField = typeBuilder.DefineField("<>_impl", typeof(Impl), FieldAttributes.Private | FieldAttributes.InitOnly); var parentField = typeBuilder.DefineField("<>_parent", typeof(IGeneratedStore), FieldAttributes.Private | FieldAttributes.InitOnly); - /*#region Converter fields - var uniqueConverterTypes = structure.Where(m => m.HasConverter).Select(m => m.Converter).Distinct().ToArray(); - var converterFields = new Dictionary(uniqueConverterTypes.Length); - - foreach (var convType in uniqueConverterTypes) - { - var field = typeBuilder.DefineField($"_{convType}", convType, - FieldAttributes.Private | FieldAttributes.InitOnly | FieldAttributes.Static); - converterFields.Add(convType, field); - - foreach (var member in structure.Where(m => m.HasConverter && m.Converter == convType)) - member.ConverterField = field; - } - #endregion - - #region Static constructor - var cctor = typeBuilder.DefineConstructor(MethodAttributes.Static, CallingConventions.Standard, Type.EmptyTypes); - { - var il = cctor.GetILGenerator(); - - foreach (var kvp in converterFields) - { - var typeCtor = kvp.Key.GetConstructor(Type.EmptyTypes); - il.Emit(OpCodes.Newobj, typeCtor); - il.Emit(OpCodes.Stsfld, kvp.Value); - } - - il.Emit(OpCodes.Ret); - } - #endregion*/ - - //CreateAndInitializeConvertersFor(type, structure); - #region Constructor var ctor = typeBuilder.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, new[] { typeof(IGeneratedStore) }); { @@ -145,10 +112,13 @@ namespace IPA.Config.Stores MethodBuilder notifyChanged = null; if (isINotifyPropertyChanged || hasNotifyAttribute) { + // we don't actually want to notify if the base class implements it if (isINotifyPropertyChanged) { - var ExistingRaisePropertyChanged = type.GetMethod("RaisePropertyChanged", (BindingFlags)int.MaxValue, null, new Type[] { typeof(string) }, Array.Empty()); - if (ExistingRaisePropertyChanged != null) + var ExistingRaisePropertyChanged = type.GetMethod("RaisePropertyChanged", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.FlattenHierarchy, + null, new Type[] { typeof(string) }, Array.Empty()); + if (ExistingRaisePropertyChanged != null && !ExistingRaisePropertyChanged.IsPrivate) { notifyChanged = typeBuilder.DefineMethod("<>NotifyChanged", MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.Final, null, new[] { typeof(string) }); @@ -164,7 +134,8 @@ namespace IPA.Config.Stores } else { - Logger.log.Critical($"Type '{type.FullName}' implements INotifyPropertyChanged but does not have a 'RaisePropertyChanged(string)' method, automatic raising of PropertyChanged event is disabled."); + Logger.log.Critical($"Type '{type.FullName}' implements INotifyPropertyChanged but does not have an accessible " + + "'RaisePropertyChanged(string)' method, automatic raising of PropertyChanged event is disabled."); } } else