mirror of
https://github.com/adamhathcock/sharpcompress.git
synced 2026-09-22 23:15:20 +00:00
Merge pull request #1172 from adamhathcock/copilot/fix-sevenzip-contiguous-streams
Fix SevenZipReader to maintain contiguous stream state for solid archives
This commit is contained in:
@@ -212,10 +212,31 @@ public class SevenZipArchive : AbstractArchive<SevenZipArchiveEntry, SevenZipVol
|
||||
public override long TotalSize =>
|
||||
_database?._packSizes.Aggregate(0L, (total, packSize) => total + packSize) ?? 0;
|
||||
|
||||
private sealed class SevenZipReader : AbstractReader<SevenZipEntry, SevenZipVolume>
|
||||
internal sealed class SevenZipReader : AbstractReader<SevenZipEntry, SevenZipVolume>
|
||||
{
|
||||
private readonly SevenZipArchive _archive;
|
||||
private SevenZipEntry? _currentEntry;
|
||||
private Stream? _currentFolderStream;
|
||||
private CFolder? _currentFolder;
|
||||
|
||||
/// <summary>
|
||||
/// Enables internal diagnostics for tests.
|
||||
/// When disabled (default), diagnostics properties return null to avoid exposing internal state.
|
||||
/// </summary>
|
||||
internal bool DiagnosticsEnabled { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Current folder instance used to decide whether the solid folder stream should be reused.
|
||||
/// Only available when <see cref="DiagnosticsEnabled"/> is true.
|
||||
/// </summary>
|
||||
internal object? DiagnosticsCurrentFolder => DiagnosticsEnabled ? _currentFolder : null;
|
||||
|
||||
/// <summary>
|
||||
/// Current shared folder stream instance.
|
||||
/// Only available when <see cref="DiagnosticsEnabled"/> is true.
|
||||
/// </summary>
|
||||
internal Stream? DiagnosticsCurrentFolderStream =>
|
||||
DiagnosticsEnabled ? _currentFolderStream : null;
|
||||
|
||||
internal SevenZipReader(ReaderOptions readerOptions, SevenZipArchive archive)
|
||||
: base(readerOptions, ArchiveType.SevenZip) => this._archive = archive;
|
||||
@@ -231,9 +252,10 @@ public class SevenZipArchive : AbstractArchive<SevenZipArchiveEntry, SevenZipVol
|
||||
_currentEntry = dir;
|
||||
yield return dir;
|
||||
}
|
||||
// For non-directory entries, yield them without creating shared streams
|
||||
// Each call to GetEntryStream() will create a fresh decompression stream
|
||||
// to avoid state corruption issues with async operations
|
||||
// For solid archives (entries in the same folder share a compressed stream),
|
||||
// we must iterate entries sequentially and maintain the folder stream state
|
||||
// across entries in the same folder to avoid recreating the decompression
|
||||
// stream for each file, which breaks contiguous streaming.
|
||||
foreach (var entry in entries.Where(x => !x.IsDirectory))
|
||||
{
|
||||
_currentEntry = entry;
|
||||
@@ -243,19 +265,53 @@ public class SevenZipArchive : AbstractArchive<SevenZipArchiveEntry, SevenZipVol
|
||||
|
||||
protected override EntryStream GetEntryStream()
|
||||
{
|
||||
// Create a fresh decompression stream for each file (no state sharing).
|
||||
// However, the LZMA decoder has bugs in its async implementation that cause
|
||||
// state corruption even on fresh streams. The SyncOnlyStream wrapper
|
||||
// works around these bugs by forcing async operations to use sync equivalents.
|
||||
//
|
||||
// TODO: Fix the LZMA decoder async bugs (in LzmaStream, Decoder, OutWindow)
|
||||
// so this wrapper is no longer necessary.
|
||||
var entry = _currentEntry.NotNull("currentEntry is not null");
|
||||
if (entry.IsDirectory)
|
||||
{
|
||||
return CreateEntryStream(Stream.Null);
|
||||
}
|
||||
return CreateEntryStream(new SyncOnlyStream(entry.FilePart.GetCompressedStream()));
|
||||
|
||||
var filePart = (SevenZipFilePart)entry.FilePart;
|
||||
if (!filePart.Header.HasStream)
|
||||
{
|
||||
// Entries with no underlying stream (e.g., empty files or anti-items)
|
||||
// should return an empty stream, matching previous behavior.
|
||||
return CreateEntryStream(Stream.Null);
|
||||
}
|
||||
|
||||
var folder = filePart.Folder;
|
||||
// Check if we're starting a new folder - dispose old folder stream if needed
|
||||
if (folder != _currentFolder)
|
||||
{
|
||||
_currentFolderStream?.Dispose();
|
||||
_currentFolderStream = null;
|
||||
_currentFolder = folder;
|
||||
}
|
||||
|
||||
// Create the folder stream once per folder
|
||||
if (_currentFolderStream is null)
|
||||
{
|
||||
_currentFolderStream = _archive._database!.GetFolderStream(
|
||||
_archive.Volumes.Single().Stream,
|
||||
folder!,
|
||||
_archive._database.PasswordProvider
|
||||
);
|
||||
}
|
||||
|
||||
// Wrap with SyncOnlyStream to work around LZMA async bugs
|
||||
// Return a ReadOnlySubStream that reads from the shared folder stream
|
||||
return CreateEntryStream(
|
||||
new SyncOnlyStream(
|
||||
new ReadOnlySubStream(_currentFolderStream, entry.Size, leaveOpen: true)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
public override void Dispose()
|
||||
{
|
||||
_currentFolderStream?.Dispose();
|
||||
_currentFolderStream = null;
|
||||
base.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -251,4 +251,98 @@ public class SevenZipArchiveTests : ArchiveTests
|
||||
);
|
||||
Assert.False(nonSolidArchive.IsSolid);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SevenZipArchive_Solid_ExtractAllEntries_Contiguous()
|
||||
{
|
||||
// This test verifies that solid archives iterate entries as contiguous streams
|
||||
// rather than recreating the decompression stream for each entry
|
||||
var testArchive = Path.Combine(TEST_ARCHIVES_PATH, "7Zip.solid.7z");
|
||||
using var archive = SevenZipArchive.Open(testArchive);
|
||||
Assert.True(archive.IsSolid);
|
||||
|
||||
using var reader = archive.ExtractAllEntries();
|
||||
while (reader.MoveToNextEntry())
|
||||
{
|
||||
if (!reader.Entry.IsDirectory)
|
||||
{
|
||||
reader.WriteEntryToDirectory(
|
||||
SCRATCH_FILES_PATH,
|
||||
new ExtractionOptions { ExtractFullPath = true, Overwrite = true }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
VerifyFiles();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SevenZipArchive_Solid_VerifyStreamReuse()
|
||||
{
|
||||
// This test verifies that the folder stream is reused within each folder
|
||||
// and not recreated for each entry in solid archives
|
||||
var testArchive = Path.Combine(TEST_ARCHIVES_PATH, "7Zip.solid.7z");
|
||||
using var archive = SevenZipArchive.Open(testArchive);
|
||||
Assert.True(archive.IsSolid);
|
||||
|
||||
using var reader = archive.ExtractAllEntries();
|
||||
|
||||
var sevenZipReader = Assert.IsType<SevenZipArchive.SevenZipReader>(reader);
|
||||
sevenZipReader.DiagnosticsEnabled = true;
|
||||
|
||||
Stream? currentFolderStreamInstance = null;
|
||||
object? currentFolder = null;
|
||||
var entryCount = 0;
|
||||
var entriesInCurrentFolder = 0;
|
||||
var streamRecreationsWithinFolder = 0;
|
||||
|
||||
while (reader.MoveToNextEntry())
|
||||
{
|
||||
if (!reader.Entry.IsDirectory)
|
||||
{
|
||||
// Extract the entry to trigger GetEntryStream
|
||||
using var entryStream = reader.OpenEntryStream();
|
||||
var buffer = new byte[4096];
|
||||
while (entryStream.Read(buffer, 0, buffer.Length) > 0)
|
||||
{
|
||||
// Read the stream to completion
|
||||
}
|
||||
|
||||
entryCount++;
|
||||
|
||||
var folderStream = sevenZipReader.DiagnosticsCurrentFolderStream;
|
||||
var folder = sevenZipReader.DiagnosticsCurrentFolder;
|
||||
|
||||
Assert.NotNull(folderStream); // Folder stream should exist
|
||||
|
||||
// Check if we're in a new folder
|
||||
if (currentFolder == null || !ReferenceEquals(currentFolder, folder))
|
||||
{
|
||||
// Starting a new folder
|
||||
currentFolder = folder;
|
||||
currentFolderStreamInstance = folderStream;
|
||||
entriesInCurrentFolder = 1;
|
||||
}
|
||||
else
|
||||
{
|
||||
// Same folder - verify stream wasn't recreated
|
||||
entriesInCurrentFolder++;
|
||||
|
||||
if (!ReferenceEquals(currentFolderStreamInstance, folderStream))
|
||||
{
|
||||
// Stream was recreated within the same folder - this is the bug we're testing for!
|
||||
streamRecreationsWithinFolder++;
|
||||
}
|
||||
|
||||
currentFolderStreamInstance = folderStream;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Verify we actually tested multiple entries
|
||||
Assert.True(entryCount > 1, "Test should have multiple entries to verify stream reuse");
|
||||
|
||||
// The critical check: within a single folder, the stream should NEVER be recreated
|
||||
Assert.Equal(0, streamRecreationsWithinFolder); // Folder stream should remain the same for all entries in the same folder
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user