Skip to content

Commit ca4eea3

Browse files
NinoFlorisNinoFloris
authored
Move write state types out of generic converters and rename (#6572)
1 parent c5b7479 commit ca4eea3

9 files changed

Lines changed: 123 additions & 123 deletions

File tree

‎src/Npgsql/Internal/Converters/CompositeConverter.cs‎

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -190,15 +190,15 @@ protected override Size BindValue(in BindContext context, T value, ref object? w
190190

191191
return totalSize;
192192

193-
static WriteState RentWrapper(object boxedInstance, int fieldCount, bool anyWriteState)
193+
static CompositeWriteState RentWrapper(object boxedInstance, int fieldCount, bool anyWriteState)
194194
{
195-
var data = ArrayPool<ElementState>.Shared.Rent(fieldCount);
196-
// Stale slots from the previous pool user must read as default(ElementState) — Dispose iterates
195+
var data = ArrayPool<CompositeWriteState.FieldState>.Shared.Rent(fieldCount);
196+
// Stale slots from the previous pool user must read as default(CompositeWriteState.FieldState) — Dispose iterates
197197
// the full segment and relies on null WriteState in unfilled slots to skip them safely.
198198
Array.Clear(data, 0, fieldCount);
199-
return new WriteState
199+
return new CompositeWriteState
200200
{
201-
ArrayPool = ArrayPool<ElementState>.Shared,
201+
ArrayPool = ArrayPool<CompositeWriteState.FieldState>.Shared,
202202
Data = new(data, 0, fieldCount),
203203
AnyWriteState = anyWriteState,
204204
BoxedInstance = boxedInstance,
@@ -221,10 +221,10 @@ async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken ca
221221
// we can't recover per-field value-dependent sizes otherwise.
222222
var writeState = writer.Current.WriteState switch
223223
{
224-
WriteState ws => ws,
224+
CompositeWriteState ws => ws,
225225
null when _writeSizePrecomputed.Kind is SizeKind.Exact => null,
226226
null => throw new InvalidOperationException("Composite Write requires per-field data from BindValue when any field is variable-size."),
227-
_ => throw new InvalidCastException($"Invalid write state, expected {typeof(WriteState).FullName}.")
227+
_ => throw new InvalidCastException($"Invalid write state, expected {typeof(CompositeWriteState).FullName}.")
228228
};
229229
Debug.Assert(_bufferRequirements.Write.Kind is not SizeKind.Exact || writeState is null,
230230
"Exact-size composite must not carry write state — BindValue should have been skipped.");
@@ -249,7 +249,7 @@ async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken ca
249249
// path for fixed-size fields). The cached default writes the same bytes a stateful slot would
250250
// have, with no per-slot disposal obligation. Variable-size composites must populate every
251251
// slot during BindValue, so data[i].Converter is never null on the slow path.
252-
ElementState elementState;
252+
CompositeWriteState.FieldState elementState;
253253
if (data?[i] is { Converter: not null } state)
254254
elementState = state;
255255
else
@@ -273,42 +273,43 @@ async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken ca
273273
}
274274
}
275275

276-
struct ElementState
277-
{
278-
public Size Size;
279-
public object? WriteState;
280-
public PgConverter? Converter;
281-
public Size BufferRequirement;
282-
}
276+
}
283277

284-
class WriteState : IDisposable
285-
{
286-
public required ArrayPool<ElementState>? ArrayPool { get; set; }
287-
public required ArraySegment<ElementState> Data { get; set; }
288-
public required bool AnyWriteState { get; set; }
289-
public required object BoxedInstance { get; set; }
290-
int _disposed;
278+
file sealed class CompositeWriteState : IDisposable
279+
{
280+
public required ArrayPool<FieldState>? ArrayPool { get; set; }
281+
public required ArraySegment<FieldState> Data { get; set; }
282+
public required bool AnyWriteState { get; set; }
283+
public required object BoxedInstance { get; set; }
284+
int _disposed;
291285

292-
public void Dispose()
286+
public void Dispose()
287+
{
288+
// Atomic idempotency guard — double-dispose returns the rented FieldState[] to the pool
289+
// twice. Atomic catches concurrent disposal too, important once states become reusable.
290+
if (Interlocked.Exchange(ref _disposed, 1) != 0)
293291
{
294-
// Atomic idempotency guard — double-dispose returns the rented ElementState[] to the pool
295-
// twice. Atomic catches concurrent disposal too, important once states become reusable.
296-
if (Interlocked.Exchange(ref _disposed, 1) != 0)
297-
{
298-
Debug.Assert(false, "CompositeConverter.WriteState double-dispose detected — caller violated lifecycle contract.");
299-
return;
300-
}
292+
Debug.Assert(false, $"{nameof(CompositeWriteState)} double-dispose detected — caller violated lifecycle contract.");
293+
return;
294+
}
301295

302-
if (Data.Array is not { } array)
303-
return;
296+
if (Data.Array is not { } array)
297+
return;
304298

305-
if (AnyWriteState)
306-
for (var i = Data.Offset; i < Data.Offset + Data.Count; i++)
307-
if (array[i].WriteState is IDisposable disposable)
308-
disposable.Dispose();
299+
if (AnyWriteState)
300+
for (var i = Data.Offset; i < Data.Offset + Data.Count; i++)
301+
if (array[i].WriteState is IDisposable disposable)
302+
disposable.Dispose();
309303

310-
Array.Clear(array, Data.Offset, Data.Count);
311-
ArrayPool?.Return(array);
312-
}
304+
Array.Clear(array, Data.Offset, Data.Count);
305+
ArrayPool?.Return(array);
306+
}
307+
308+
public struct FieldState
309+
{
310+
public Size Size;
311+
public object? WriteState;
312+
public PgConverter? Converter;
313+
public Size BufferRequirement;
313314
}
314315
}

‎src/Npgsql/Internal/Converters/HstoreConverter.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ protected override Size BindValue(in BindContext context, T value, ref object? w
2929
var arrayPool = ArrayPool<(Size Size, object? WriteState)>.Shared;
3030
var data = arrayPool.Rent(value.Count * 2);
3131
Array.Clear(data, 0, value.Count * 2);
32-
writeState = new WriteState
32+
writeState = new HstoreWriteState
3333
{
3434
ArrayPool = arrayPool,
3535
Data = new(data, 0, value.Count * 2),
@@ -101,15 +101,15 @@ async ValueTask<T> Read(bool async, PgReader reader, CancellationToken cancellat
101101

102102
async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken cancellationToken)
103103
{
104-
if (writer.Current.WriteState is not WriteState && value.Count is not 0)
105-
throw new InvalidCastException($"Invalid write state, expected {typeof(WriteState).FullName}.");
104+
if (writer.Current.WriteState is not HstoreWriteState && value.Count is not 0)
105+
throw new InvalidCastException($"Invalid write state, expected {typeof(HstoreWriteState).FullName}.");
106106

107107
// Number of lengths (count, key length, value length).
108108
if (writer.ShouldFlush(sizeof(int)))
109109
await writer.Flush(async, cancellationToken).ConfigureAwait(false);
110110
writer.WriteInt32(value.Count);
111111

112-
if (value.Count is 0 || writer.Current.WriteState is not WriteState writeState)
112+
if (value.Count is 0 || writer.Current.WriteState is not HstoreWriteState writeState)
113113
return;
114114

115115
var data = writeState.Data;
@@ -150,7 +150,6 @@ async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken ca
150150
}
151151
}
152152

153-
sealed class WriteState : MultiWriteState
154-
{
155-
}
156153
}
154+
155+
file sealed class HstoreWriteState : MultiWriteState;

src/Npgsql/Internal/Converters/ObjectConverter.cs renamed to src/Npgsql/Internal/Converters/LateBindingConverter.cs

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,17 @@
66

77
namespace Npgsql.Internal;
88

9-
sealed class ObjectConverter : PgStreamingConverter<object>
9+
sealed class LateBindingConverter : PgStreamingConverter<object>
1010
{
11-
public ObjectConverter() => HandleDbNull = true;
11+
public LateBindingConverter() => HandleDbNull = true;
1212

1313
protected override bool IsDbNullValue(object? value, object? writeState)
1414
{
1515
var (concreteTypeInfo, effectiveState) = writeState switch
1616
{
1717
PgConcreteTypeInfo info => (info, (object?)null),
18-
WriteState ws => (ws.ConcreteTypeInfo, ws.EffectiveState),
19-
_ => throw new InvalidOperationException("writeState cannot be null, LateBoundTypeInfoProvider is expected to pre-populate it with concrete type info.")
18+
LateBindingWriteState ws => (ws.ConcreteTypeInfo, ws.EffectiveState),
19+
_ => throw new InvalidOperationException("writeState cannot be null, LateBindingTypeInfoProvider is expected to pre-populate it with concrete type info.")
2020
};
2121

2222
return concreteTypeInfo.Converter.IsDbNullAsObject(value, effectiveState);
@@ -30,7 +30,7 @@ protected override Size BindValue(in BindContext context, object value, ref obje
3030
var (concreteTypeInfo, effectiveState) = writeState switch
3131
{
3232
PgConcreteTypeInfo info => (info, (object?)null),
33-
WriteState state => (state.ConcreteTypeInfo, state.EffectiveState),
33+
LateBindingWriteState state => (state.ConcreteTypeInfo, state.EffectiveState),
3434
_ => throw new InvalidOperationException("Invalid state")
3535
};
3636

@@ -43,7 +43,7 @@ protected override Size BindValue(in BindContext context, object value, ref obje
4343
// Null the wrapper's EffectiveState before handoff. Inner BindAsObject's framework safety net
4444
// disposes via our local ref on throw and nulls the local; the wrapper would otherwise hold a
4545
// dangling reference to the same object, double-disposing through outer Bind's catch.
46-
if (writeState is WriteState before)
46+
if (writeState is LateBindingWriteState before)
4747
before.EffectiveState = null;
4848

4949
var result = concreteTypeInfo.Converter.BindAsObject(
@@ -52,10 +52,10 @@ protected override Size BindValue(in BindContext context, object value, ref obje
5252
ref effectiveState);
5353
if (effectiveState is not null)
5454
{
55-
if (writeState is WriteState s)
55+
if (writeState is LateBindingWriteState s)
5656
s.EffectiveState = effectiveState;
5757
else
58-
writeState = new WriteState { ConcreteTypeInfo = concreteTypeInfo, EffectiveState = effectiveState };
58+
writeState = new LateBindingWriteState { ConcreteTypeInfo = concreteTypeInfo, EffectiveState = effectiveState };
5959
}
6060

6161
return result;
@@ -72,7 +72,7 @@ async ValueTask Write(bool async, PgWriter writer, object value, CancellationTok
7272
var (concreteTypeInfo, effectiveState) = writer.Current.WriteState switch
7373
{
7474
PgConcreteTypeInfo info => (info, (object?)null),
75-
WriteState state => (state.ConcreteTypeInfo, state.EffectiveState),
75+
LateBindingWriteState state => (state.ConcreteTypeInfo, state.EffectiveState),
7676
_ => throw new InvalidOperationException("Invalid state")
7777
};
7878

@@ -82,37 +82,13 @@ async ValueTask Write(bool async, PgWriter writer, object value, CancellationTok
8282
using var _ = await writer.BeginNestedWrite(async, writeRequirement, writer.Current.Size.Value, effectiveState, cancellationToken).ConfigureAwait(false);
8383
await concreteTypeInfo.Converter.WriteAsObject(async, writer, value, cancellationToken).ConfigureAwait(false);
8484
}
85-
86-
internal sealed class WriteState : IDisposable
87-
{
88-
public required PgConcreteTypeInfo ConcreteTypeInfo { get; init; }
89-
public required object? EffectiveState { get; set; }
90-
int _disposed;
91-
92-
// EffectiveState may hold a pooled WriteState from the underlying concrete converter
93-
// (composite, array, etc.). The outer DisposeWriteState on PgTypeInfo only sees this
94-
// wrapper, so the wrapper is responsible for cascading disposal to the inner state.
95-
public void Dispose()
96-
{
97-
// Atomic idempotency guard — EffectiveState may be pool-backed; cascading double-dispose
98-
// corrupts downstream pools. Atomic catches concurrent disposal too.
99-
if (Interlocked.Exchange(ref _disposed, 1) != 0)
100-
{
101-
Debug.Assert(false, "ObjectConverter.WriteState double-dispose detected — caller violated lifecycle contract.");
102-
return;
103-
}
104-
105-
if (EffectiveState is IDisposable disposable)
106-
disposable.Dispose();
107-
}
108-
}
10985
}
11086

11187
// TODO the goal is to allow this provider to return the underlying converter type info, but we're not there yet.
112-
// At that point we don't need the ObjectConverter any longer.
113-
sealed class LateBoundTypeInfoProvider(PgSerializerOptions options, PgTypeId typeId) : PgConcreteTypeInfoProvider<object>
88+
// At that point we don't need the LateBindingConverter any longer.
89+
sealed class LateBindingTypeInfoProvider(PgSerializerOptions options, PgTypeId typeId) : PgConcreteTypeInfoProvider<object>
11490
{
115-
readonly PgConcreteTypeInfo _defaultConcreteTypeInfo = new(options, new ObjectConverter(), typeId);
91+
readonly PgConcreteTypeInfo _defaultConcreteTypeInfo = new(options, new LateBindingConverter(), typeId);
11692

11793
protected override PgConcreteTypeInfo GetDefaultCore(PgTypeId? pgTypeId)
11894
{
@@ -139,11 +115,35 @@ protected override PgConcreteTypeInfo GetForValueCore(ProviderValueContext conte
139115
// cascades to EffectiveState; PgConcreteTypeInfo (non-IDisposable) is a long-lived cached
140116
// instance so the no-wrapper branch is naturally safe.
141117
writeState = effectiveState is not null
142-
? new ObjectConverter.WriteState { ConcreteTypeInfo = concreteTypeInfo, EffectiveState = effectiveState }
118+
? new LateBindingWriteState { ConcreteTypeInfo = concreteTypeInfo, EffectiveState = effectiveState }
143119
: concreteTypeInfo;
144120
if (!concreteTypeInfo.SupportsWriting)
145121
AdoSerializerHelpers.ThrowWritingNotSupported(valueType, options, concreteTypeInfo.PgTypeId, resolved: true);
146122

147123
return GetDefault(context.ExpectedPgTypeId);
148124
}
149125
}
126+
127+
file sealed class LateBindingWriteState : IDisposable
128+
{
129+
public required PgConcreteTypeInfo ConcreteTypeInfo { get; init; }
130+
public required object? EffectiveState { get; set; }
131+
int _disposed;
132+
133+
// EffectiveState may hold a pooled write state from the underlying concrete converter
134+
// (composite, array, etc.). The outer DisposeWriteState on PgTypeInfo only sees this
135+
// wrapper, so the wrapper is responsible for cascading disposal to the inner state.
136+
public void Dispose()
137+
{
138+
// Atomic idempotency guard — EffectiveState may be pool-backed; cascading double-dispose
139+
// corrupts downstream pools. Atomic catches concurrent disposal too.
140+
if (Interlocked.Exchange(ref _disposed, 1) != 0)
141+
{
142+
Debug.Assert(false, $"{nameof(LateBindingWriteState)} double-dispose detected — caller violated lifecycle contract.");
143+
return;
144+
}
145+
146+
if (EffectiveState is IDisposable disposable)
147+
disposable.Dispose();
148+
}
149+
}

‎src/Npgsql/Internal/Converters/MultirangeConverter.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ protected override Size BindValue(in BindContext context, T value, ref object? w
7676
var arrayPool = ArrayPool<(Size Size, object? WriteState)>.Shared;
7777
var data = arrayPool.Rent(value.Count);
7878
Array.Clear(data, 0, value.Count);
79-
var state = new WriteState
79+
var state = new MultirangeWriteState
8080
{
8181
ArrayPool = arrayPool,
8282
Data = new(data, 0, value.Count),
@@ -109,7 +109,7 @@ public override ValueTask WriteAsync(PgWriter writer, T value, CancellationToken
109109

110110
async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken cancellationToken)
111111
{
112-
if (writer.Current.WriteState is not WriteState writeState)
112+
if (writer.Current.WriteState is not MultirangeWriteState writeState)
113113
throw new InvalidCastException($"Invalid state {writer.Current.WriteState?.GetType().FullName}.");
114114

115115
if (writer.ShouldFlush(sizeof(int)))
@@ -140,7 +140,6 @@ async ValueTask Write(bool async, PgWriter writer, T value, CancellationToken ca
140140
}
141141
}
142142

143-
sealed class WriteState : MultiWriteState
144-
{
145-
}
146143
}
144+
145+
file sealed class MultirangeWriteState : MultiWriteState;

0 commit comments

Comments
 (0)