Skip to content

Commit 9ece117

Browse files
HirogenHirogen
authored
Merge pull request #641 from LogExperts/closing_the_block_detach_seam_leak
Lift char-block detachment onto the reader interface
2 parents c80bc54 + 095c818 commit 9ece117

8 files changed

Lines changed: 79 additions & 75 deletions

File tree

‎src/LogExpert.Core/Classes/Log/LogfileReader.cs‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,11 @@ public ILogLineMemory[] GetLogLineMemories (int startLine, int count)
909909
var availableInBuffer = logBufferEntry.Buffer.LineCount - bufferOffset;
910910
var toCopy = Math.Min(count - filled, availableInBuffer);
911911

912+
if (bufferOffset < 0 || toCopy <= 0)
913+
{
914+
break;
915+
}
916+
912917
for (var i = 0; i < toCopy; i++)
913918
{
914919
result[filled + i] = logBufferEntry.Buffer.GetLineMemoryOfBlock(bufferOffset + i);
@@ -1256,16 +1261,9 @@ private void ReadToBufferList (ILogFileInfo logFileInfo, long filePos, int start
12561261

12571262
logBuffer.Size = filePos - logBuffer.StartPos;
12581263

1259-
// Detach char blocks from the reader's allocator and attach to the completed buffer.
1264+
// Detach char blocks from the reader and attach to the completed buffer.
12601265
// Must happen before Monitor.Exit so the buffer is still exclusively owned.
1261-
if (reader is PositionAwareStreamReaderSystem systemDetachBlockReader)
1262-
{
1263-
logBuffer.AttachCharBlocks(systemDetachBlockReader.BlockAllocator.DetachBlocks());
1264-
}
1265-
else if (reader is PositionAwareStreamReaderDirect directReader)
1266-
{
1267-
logBuffer.AttachCharBlocks(directReader.DetachBlocks());
1268-
}
1266+
logBuffer.AttachCharBlocks(reader.DetachCharBlocks());
12691267

12701268
Monitor.Exit(logBuffer);
12711269
try
@@ -1297,14 +1295,7 @@ private void ReadToBufferList (ILogFileInfo logFileInfo, long filePos, int start
12971295
logBuffer.Size = filePos - logBuffer.StartPos;
12981296

12991297
// Attach remaining blocks to the final buffer
1300-
if (reader is PositionAwareStreamReaderSystem systemDetachBlockReader2)
1301-
{
1302-
logBuffer.AttachCharBlocks(systemDetachBlockReader2.BlockAllocator.DetachBlocks());
1303-
}
1304-
else if (reader is PositionAwareStreamReaderDirect directReader)
1305-
{
1306-
logBuffer.AttachCharBlocks(directReader.DetachBlocks());
1307-
}
1298+
logBuffer.AttachCharBlocks(reader.DetachCharBlocks());
13081299
}
13091300
finally
13101301
{
@@ -1427,14 +1418,7 @@ private void ReReadBuffer (LogBuffer logBuffer)
14271418
}
14281419

14291420
// Attach char blocks from the reader to the re-read buffer
1430-
if (reader is PositionAwareStreamReaderSystem systemReader)
1431-
{
1432-
logBuffer.AttachCharBlocks(systemReader.BlockAllocator.DetachBlocks());
1433-
}
1434-
else if (reader is PositionAwareStreamReaderDirect directReader)
1435-
{
1436-
logBuffer.AttachCharBlocks(directReader.DetachBlocks());
1437-
}
1421+
logBuffer.AttachCharBlocks(reader.DetachCharBlocks());
14381422

14391423
if (maxLinesCount != logBuffer.LineCount)
14401424
{

‎src/LogExpert.Core/Classes/Log/Streamreaders/PositionAwareStreamReaderDirect.cs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using System.Buffers;
22
using System.Text;
33

4-
using LogExpert.Core.Classes.Log.Buffers;
54
using LogExpert.Core.Entities;
65
using LogExpert.Core.Interfaces;
76

@@ -156,17 +155,11 @@ public void ReturnMemory (ReadOnlyMemory<char> memory)
156155
// Bulk return via DetachBlocks()/Dispose(). Individual return not needed.
157156
}
158157

159-
/// <summary>
160-
/// Gets the block allocator for compatibility with the DetachBlocks pattern.
161-
/// This reader manages its own blocks directly rather than through CharBlockAllocator.
162-
/// </summary>
163-
public CharBlockAllocator? BlockAllocator => null;
164-
165158
/// <summary>
166159
/// Detaches completed blocks (fully scanned) for transfer to the LogBuffer.
167160
/// The current _readBlock (partially scanned) stays with the reader.
168161
/// </summary>
169-
public List<char[]> DetachBlocks ()
162+
public List<char[]> DetachCharBlocks ()
170163
{
171164
// Nothing to detach: no completed blocks and no lines were scanned from the current block.
172165
if (_completedBlocks.Count == 0 && _scanOffset == 0)

‎src/LogExpert.Core/Classes/Log/Streamreaders/PositionAwareStreamReaderLegacy.cs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ public class PositionAwareStreamReaderLegacy (Stream stream, EncodingOptions enc
1919

2020
#region Properties
2121

22-
public CharBlockAllocator BlockAllocator
22+
private CharBlockAllocator BlockAllocator
2323
{
2424
get => field ??= new CharBlockAllocator();
25-
private set;
25+
set;
2626
}
2727

2828
#endregion
@@ -47,7 +47,20 @@ public bool TryReadLine (out ReadOnlyMemory<char> lineMemory)
4747

4848
public void ReturnMemory (ReadOnlyMemory<char> memory)
4949
{
50-
// Bulk return via BlockAllocator.DetachBlocks() when the LogBuffer is evicted.
50+
// Bulk return via DetachCharBlocks() when the LogBuffer is evicted, or Dispose().
51+
}
52+
53+
public List<char[]> DetachCharBlocks () => BlockAllocator.DetachBlocks();
54+
55+
protected override void Dispose (bool disposing)
56+
{
57+
if (disposing)
58+
{
59+
BlockAllocator?.Dispose();
60+
BlockAllocator = null;
61+
}
62+
63+
base.Dispose(disposing);
5164
}
5265

5366
public override string ReadLine ()

‎src/LogExpert.Core/Classes/Log/Streamreaders/PositionAwareStreamReaderSystem.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@ public PositionAwareStreamReaderSystem (Stream stream, EncodingOptions encodingO
3636
#region Properties
3737

3838
/// <summary>
39-
/// Gets or creates the block allocator used by this reader instance.
40-
/// The caller can detach the blocks after reading a buffer's worth of lines.
39+
/// The block allocator used by this reader instance. Blocks are handed to the owning buffer via
40+
/// <see cref="DetachCharBlocks"/>; ownership of the allocator does not cross the reader seam.
4141
/// </summary>
42-
public CharBlockAllocator BlockAllocator
42+
private CharBlockAllocator BlockAllocator
4343
{
4444
get => field ??= new CharBlockAllocator();
45-
private set;
45+
set;
4646
}
4747

4848
#endregion
@@ -112,10 +112,12 @@ public bool TryReadLine (out ReadOnlyMemory<char> lineMemory)
112112
/// </summary>
113113
public void ReturnMemory (ReadOnlyMemory<char> memory)
114114
{
115-
// Bulk return via BlockAllocator.DetachBlocks() or Dispose().
115+
// Bulk return via DetachCharBlocks() or Dispose().
116116
// Individual per-line return is not needed with block-based allocation.
117117
}
118118

119+
public List<char[]> DetachCharBlocks () => BlockAllocator.DetachBlocks();
120+
119121
#endregion
120122

121123
#region Private Methods

‎src/LogExpert.Core/Interfaces/ILogStreamReaderMemory.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,19 @@ public interface ILogStreamReaderMemory : ILogStreamReader
3636
/// call this method multiple times for the same memory, but only the first call will have an effect.
3737
/// </remarks>
3838
void ReturnMemory (ReadOnlyMemory<char> memory);
39+
40+
/// <summary>
41+
/// Detaches the completed character blocks that back the <see cref="ReadOnlyMemory{Char}"/> slices handed out by
42+
/// <see cref="TryReadLine"/>, transferring their ownership to the caller (the <c>LogBuffer</c> that owns those
43+
/// lines).
44+
/// </summary>
45+
/// <returns>
46+
/// The list of blocks the reader has filled. The caller owns them until every slice into them is released, at
47+
/// which point they are returned to the shared pool. Readers that have nothing to hand over return an empty list.
48+
/// </returns>
49+
/// <remarks>
50+
/// Call this after a buffer's worth of lines has been read and before the reader continues filling the next
51+
/// buffer. The reader retains any partially-scanned block so reading can continue seamlessly.
52+
/// </remarks>
53+
List<char[]> DetachCharBlocks ();
3954
}

‎src/LogExpert.Tests/StreamReaderTests/LogStreamReaderTest.cs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void TryReadLine_ReturnsBlockBackedMemory_NotStringBacked ()
178178
}
179179

180180
[Test]
181-
public void TryReadLine_BlockAllocatorHasBlocks_AfterReading ()
181+
public void TryReadLine_DetachCharBlocks_ReturnsFilledBlocks_AfterReading ()
182182
{
183183
var text = "Line 1\nLine 2\nLine 3\n";
184184
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(text));
@@ -189,12 +189,12 @@ public void TryReadLine_BlockAllocatorHasBlocks_AfterReading ()
189189
// Intentionally empty: consume all lines to advance reader state.
190190
}
191191

192-
// The allocator should have at least 1 block
193-
Assert.That(reader.BlockAllocator.BlockCount, Is.GreaterThanOrEqualTo(1));
192+
// Reading rented at least one block, exposed only through the seam.
193+
Assert.That(reader.DetachCharBlocks(), Has.Count.GreaterThanOrEqualTo(1));
194194
}
195195

196196
[Test]
197-
public void TryReadLine_DetachBlocks_TransfersOwnership ()
197+
public void TryReadLine_DetachCharBlocks_TransfersOwnership ()
198198
{
199199
var text = "Line 1\nLine 2\nLine 3\n";
200200
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(text));
@@ -205,11 +205,8 @@ public void TryReadLine_DetachBlocks_TransfersOwnership ()
205205
// Intentionally empty: consume all lines to advance reader state.
206206
}
207207

208-
var blocks = reader.BlockAllocator.DetachBlocks();
208+
var blocks = reader.DetachCharBlocks();
209209
Assert.That(blocks, Has.Count.GreaterThanOrEqualTo(1));
210-
211-
// After detach, allocator should have a fresh block
212-
Assert.That(reader.BlockAllocator.BlockCount, Is.EqualTo(1));
213210
}
214211

215212
[Test]

‎src/LogExpert.Tests/StreamReaderTests/PositionAwareStreamReaderDirectTests.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ public void ContentParity_LargeFile ()
258258

259259
#endregion
260260

261-
#region DetachBlocks
261+
#region DetachCharBlocks
262262

263263
[Test]
264-
public void DetachBlocks_ReturnsCompletedBlocks ()
264+
public void DetachCharBlocks_ReturnsCompletedBlocks ()
265265
{
266266
// Create enough content to fill multiple blocks
267267
var sb = new StringBuilder();
@@ -279,11 +279,11 @@ public void DetachBlocks_ReturnsCompletedBlocks ()
279279
// Intentionally empty: consume all lines to advance reader state.
280280
}
281281

282-
var blocks = reader.DetachBlocks();
282+
var blocks = reader.DetachCharBlocks();
283283
Assert.That(blocks.Count, Is.GreaterThan(0), "Should have completed blocks to detach");
284284

285285
// Second detach should be empty
286-
var blocks2 = reader.DetachBlocks();
286+
var blocks2 = reader.DetachCharBlocks();
287287
Assert.That(blocks2.Count, Is.EqualTo(0));
288288
}
289289

@@ -506,7 +506,7 @@ public void TryReadLine_LineLongerThanBlockSize_WithMaxLineLengthTruncation ()
506506
}
507507

508508
[Test]
509-
public void TryReadLine_LineLongerThanBlockSize_DetachBlocksAfterEachLine ()
509+
public void TryReadLine_LineLongerThanBlockSize_DetachCharBlocksAfterEachLine ()
510510
{
511511
RunWithTimeout(() =>
512512
{
@@ -521,12 +521,12 @@ public void TryReadLine_LineLongerThanBlockSize_DetachBlocksAfterEachLine ()
521521

522522
Assert.That(reader.TryReadLine(out var line1), Is.True);
523523
Assert.That(line1.Span.ToString(), Is.EqualTo("line1"));
524-
var blocks1 = reader.DetachBlocks();
524+
var blocks1 = reader.DetachCharBlocks();
525525
Assert.That(blocks1.Count, Is.GreaterThan(0));
526526

527527
Assert.That(reader.TryReadLine(out var line2), Is.True);
528528
Assert.That(line2.Length, Is.EqualTo(lineLength));
529-
var blocks2 = reader.DetachBlocks();
529+
var blocks2 = reader.DetachCharBlocks();
530530
Assert.That(blocks2.Count, Is.GreaterThan(0));
531531

532532
Assert.That(reader.TryReadLine(out var line3), Is.True);
@@ -537,7 +537,7 @@ public void TryReadLine_LineLongerThanBlockSize_DetachBlocksAfterEachLine ()
537537
}
538538

539539
[Test]
540-
public void TryReadLine_LineLongerThanBlockSize_DetachBlocksWithLargeTail ()
540+
public void TryReadLine_LineLongerThanBlockSize_DetachCharBlocksWithLargeTail ()
541541
{
542542
RunWithTimeout(() =>
543543
{
@@ -573,14 +573,14 @@ public void TryReadLine_LineLongerThanBlockSize_DetachBlocksWithLargeTail ()
573573
// Read and detach the prefix (resets reader to fresh BLOCK_SIZE buffer)
574574
Assert.That(reader.TryReadLine(out var line1), Is.True);
575575
Assert.That(line1.Span.ToString(), Is.EqualTo("prefix"));
576-
_ = reader.DetachBlocks();
576+
_ = reader.DetachCharBlocks();
577577

578578
// Read the long line — this grows the buffer to 131072
579579
Assert.That(reader.TryReadLine(out var line2), Is.True);
580580
Assert.That(line2.Length, Is.EqualTo(longLineLength));
581581

582582
// THIS IS THE CRITICAL CALL: DetachBlocks must handle tail > BLOCK_SIZE
583-
var blocks = reader.DetachBlocks();
583+
var blocks = reader.DetachCharBlocks();
584584
Assert.That(blocks.Count, Is.GreaterThan(0));
585585

586586
// Verify we can still read subsequent lines correctly

‎src/PluginRegistry/PluginHashGenerator.Generated.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,35 +10,35 @@ public static partial class PluginValidator
1010
{
1111
/// <summary>
1212
/// Gets pre-calculated SHA256 hashes for built-in plugins.
13-
/// Generated: 2026-06-25 12:45:02 UTC
13+
/// Generated: 2026-06-25 14:06:29 UTC
1414
/// Configuration: Release
1515
/// Plugin count: 21
1616
/// </summary>
1717
public static Dictionary<string, string> GetBuiltInPluginHashes()
1818
{
1919
return new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
2020
{
21-
["AutoColumnizer.dll"] = "97CB14C8E6F7A714A74E92152CD947B2DF26742AC4C6F1550366B5C2586FB7F8",
21+
["AutoColumnizer.dll"] = "E6BBCB994A140BC595E67D19A7E6E9975469718628C5BA8E423DCF13E076FA81",
2222
["BouncyCastle.Cryptography.dll"] = "E5EEAF6D263C493619982FD3638E6135077311D08C961E1FE128F9107D29EBC6",
2323
["BouncyCastle.Cryptography.dll (x86)"] = "E5EEAF6D263C493619982FD3638E6135077311D08C961E1FE128F9107D29EBC6",
24-
["CsvColumnizer.dll"] = "9C9F3914A4ECD1E9A45A0B87845E942162DB8203BCEF05FCA8E8E4A6C10D6AC2",
25-
["CsvColumnizer.dll (x86)"] = "9C9F3914A4ECD1E9A45A0B87845E942162DB8203BCEF05FCA8E8E4A6C10D6AC2",
26-
["DefaultPlugins.dll"] = "8EA6195A67840581DA4987B3B2893C68D8DC6DCA919B180896258386B9DB2713",
27-
["FlashIconHighlighter.dll"] = "5095E95DD6AEED1747BB6CF259D550B2B3D530B6F925C47A7458ECEDBF2EBC60",
28-
["GlassfishColumnizer.dll"] = "66730D4646D90568BFDCE500363CF2215B52F538B1E9172A78ECC6CAE0DD1C27",
29-
["JsonColumnizer.dll"] = "7C064A5DE3A970C79518280D43E8C041322B4FBC27D0D8A77BC387DFD738936F",
30-
["JsonCompactColumnizer.dll"] = "5EEAFD0882DD1DBE5F0FFF92A65580737B775E43CA7F3A0DC4EFDE6A28B0DE5E",
31-
["Log4jXmlColumnizer.dll"] = "C8458E855FE59F799E2394500BF4991A7E5E7F8AA6B030E2B7A733322E2AFAEF",
32-
["LogExpert.Resources.dll"] = "F765E294A0EEEC525FA8992D22B1A666880CE2D0F720EFAF154D01EBB7A8AEED",
24+
["CsvColumnizer.dll"] = "024575D2CBD09E58AE7CFF058B92C3785006AD2280301B0DE0724409934F4BCA",
25+
["CsvColumnizer.dll (x86)"] = "024575D2CBD09E58AE7CFF058B92C3785006AD2280301B0DE0724409934F4BCA",
26+
["DefaultPlugins.dll"] = "19B0A154B48F99A616A8468E8AC45ACCF86B4402F9F4E7DC860C6FE1D7D92B13",
27+
["FlashIconHighlighter.dll"] = "4A488895316B01053DD3287604DC4344CCEA918EB09EB664009BC419F29FC021",
28+
["GlassfishColumnizer.dll"] = "93DF4EEDC1A695F06E7E09AB483782FA0FDC867BED61FB63BD25458389FA8E0E",
29+
["JsonColumnizer.dll"] = "C9538CA60C8E1ADFEAC74C75CAE7F1627311FF500804C6FD8B2EFEF068C6FFFE",
30+
["JsonCompactColumnizer.dll"] = "1F30ABDC119D4459A2272D96080F4BF0683D7420A624CC256C2F07EEE02B192A",
31+
["Log4jXmlColumnizer.dll"] = "D99A998A0E0C3969EB8B49CB6E8BD9CCC934641EEE18852B202244C837ADC7F5",
32+
["LogExpert.Resources.dll"] = "43FB2C2C6D53D1E2CDB0D8FD663AEE5CF3E4FC51A05D69DBFBBF2123E8F67935",
3333
["Microsoft.Extensions.DependencyInjection.Abstractions.dll"] = "67FA4325000DB017DC0C35829B416F024F042D24EFB868BCF17A895EE6500A93",
3434
["Microsoft.Extensions.DependencyInjection.Abstractions.dll (x86)"] = "67FA4325000DB017DC0C35829B416F024F042D24EFB868BCF17A895EE6500A93",
3535
["Microsoft.Extensions.Logging.Abstractions.dll"] = "BB853130F5AFAF335BE7858D661F8212EC653835100F5A4E3AA2C66A4D4F685D",
3636
["Microsoft.Extensions.Logging.Abstractions.dll (x86)"] = "BB853130F5AFAF335BE7858D661F8212EC653835100F5A4E3AA2C66A4D4F685D",
37-
["RegexColumnizer.dll"] = "009C8C7E56BB89C9448091A294894FE69C661DF35F9082CD2A092EEAF85E23A7",
38-
["SftpFileSystem.dll"] = "74624F9B6659F83B53B7FEC45267345B84BE3DB13C811E04926884E65C32D56E",
39-
["SftpFileSystem.dll (x86)"] = "03A1F8E1A3DF8915404A2653FE55B39B196EFD19C6DE42586D0B9822EFFBC617",
40-
["SftpFileSystem.Resources.dll"] = "09F468603D3EAF56B13058BCE104CF9EA3F7B757352E9199D54486C8734AC47B",
41-
["SftpFileSystem.Resources.dll (x86)"] = "09F468603D3EAF56B13058BCE104CF9EA3F7B757352E9199D54486C8734AC47B",
37+
["RegexColumnizer.dll"] = "926229B7C704E367BA768E44981D133E6B26356CCEC4BCF0108EC77B2521C33A",
38+
["SftpFileSystem.dll"] = "066BDA9245EDCECCD49E3C36B6ACC05E52C8C34D25A1A9406E403987E603FC57",
39+
["SftpFileSystem.dll (x86)"] = "A30EB5C2D2E8A403C0ADF179AB626278689198E64935DA8BA108B33FB164AECA",
40+
["SftpFileSystem.Resources.dll"] = "E68455B141D714A19779E717558DEEA6C9A9829ECF5A3C5EDCD65F18C06E6D33",
41+
["SftpFileSystem.Resources.dll (x86)"] = "E68455B141D714A19779E717558DEEA6C9A9829ECF5A3C5EDCD65F18C06E6D33",
4242

4343
};
4444
}

0 commit comments

Comments
 (0)