ZenStoreWriter bug when computing only Hash

Development Pipeline & Plugins
, ,
1

Looking at FZenStoreWriter::CommitPackageInternal(), the path that only computes the hashes for the packages is missing the chunk hashes for the package data.

Here’s the relevant code:

if (bWriteOp)
{
...
	for (FPackageDataEntry& PkgData : PackageState->PackageData)
	{
		FCompressedBuffer Payload = PkgData.CompressedPayload.GetResult();
		if (bComputeHash)
		{
			PackageState->PackageHashes->ChunkHashes.Add(PkgData.Info.ChunkId, Payload.GetRawHash());
		}
...
}
else if (bComputeHash)
{
...
		for (FPackageDataEntry& PkgData : PackageState->PackageData)
		{
			FCompressedBuffer Payload = PkgData.CompressedPayload.GetResult();
			FIoHash IoHash = Payload.GetRawHash();
			PkgHashGen.Update(IoHash.GetBytes(), sizeof(FIoHash::ByteArray));
		}
...
}

Notice that the lower for loop is missing:

PackageState->PackageHashes->ChunkHashes.Add(PkgData.Info.ChunkId, IoHash);

Which is present when writing out files and also in the BulkData and FileData loops when only computing the hashes.

Regards,

Ernesto.

[Attachment Removed]

2

Thanks, I am working on testing the fix for this.

The compute-hashes-only path actually has the correct code. The presence of the hash computation in the standard commit path is the mistake.

It was removed in CL 23793142 with this comment:

> Removing storing the hashes of chunk types other than bulk and external file, because Zen stores packed data before its hit by the PackageOptimizer under the same chunk id, so it doesn’t match the data that ends up in the iostore containers.

A later change 24343344 accidentally undid the removal from 23793142 in the standard commit path, but did keep the removal in the compute-hashes-only path.

The ChunkHashes data is currently only used in DiffAssetBulkData so it should not cause a problem to remove the package data from it.

[Attachment Removed]

3

Sorry for the late update; I submitted the fix for this in 0028d5aed7babbdbbcf133555e3b6825cb4ec6cb .

[Attachment Removed]

4

Thanks for the explanation Matt. I just noticed the computed hash would end up being different for the same package on different paths.

Ernesto.

[Attachment Removed]