Weird problem with Custom StreamReader with Read override not reading correct number of bytes during base method call
-
I have made a custom filestream class that creates a copy of a file and uses the copy as the source to rewrite the file while making changes to values that match a specific criteria. Here is the basic setup:
public class MixedCompressionStreamRewriter : FileStream
It uses two modes: Parse mode reroutes the stream into a buffer to be analyzed and potentially rewritten before it gets sent to the output stream. When parse mode is false, it simply copies the base stream to the output stream. It also allows analysis of compressed data inside the stream because the source files will often have such data. Because of these use cases I had to override the Read method as such (commented so that one can trace where everything goes wrong in a debug session): The issue occurs while executing the ReadUint32 call in the code below:
public static RecordHeaderBase FromFile(BinaryReader reader, uint internalIDMask = 0xFFFFFFFF)
{
ModRecordConsolidator consolidator = reader.BaseStream as ModRecordConsolidator;
if (consolidator != null)
consolidator.EnableParseMode = true;
RecordTypeInfo typeInfo = (RecordType)reader.ReadUInt32();
if (consolidator != null)
consolidator.EnableParseMode = false;
if (typeInfo == RecordType.TES4)
return new GameHeader(reader);
else if (typeInfo.isContainer)
return new GroupHeader(reader, typeInfo, internalIDMask);
else
return new RecordHeader(reader, typeInfo, internalIDMask);
}This call leads to the execution of the overridden read method below which is where the actual problem occurs:
public override int Read(byte[] array, int offset, int count)
{
int result;
if (compressionHeaderStream == null) // condition is true when problem occurs
{
result = base.Read(array, offset, count); // ***THIS IS WHERE THE ISSUES IS ***//
if (!parseMode)
{
long equivalentBufferLocation = MemoryBufferStart + MemoryBuffer.Length;
if (Position > equivalentBufferLocation)
{
long bufferCopyBytes = Position - equivalentBufferLocation;
if (bufferCopyBytes > int.MaxValue)
throw new OverflowException();
offset += (count - (int)bufferCopyBytes);
MemoryBuffer.Write(array, offset, (int)bufferCopyBytes);
}
if (MaxBufferSize != 0)
if (MemoryBuffer.Length >= MaxBufferSize)
FlushToOutput();
}
}
else if (MemoryBuffer == null)
result = base.Read(array, offset, count);
else
result = MemoryBuffer.Read(array, offset, count);
return result;
}For some
-
I have made a custom filestream class that creates a copy of a file and uses the copy as the source to rewrite the file while making changes to values that match a specific criteria. Here is the basic setup:
public class MixedCompressionStreamRewriter : FileStream
It uses two modes: Parse mode reroutes the stream into a buffer to be analyzed and potentially rewritten before it gets sent to the output stream. When parse mode is false, it simply copies the base stream to the output stream. It also allows analysis of compressed data inside the stream because the source files will often have such data. Because of these use cases I had to override the Read method as such (commented so that one can trace where everything goes wrong in a debug session): The issue occurs while executing the ReadUint32 call in the code below:
public static RecordHeaderBase FromFile(BinaryReader reader, uint internalIDMask = 0xFFFFFFFF)
{
ModRecordConsolidator consolidator = reader.BaseStream as ModRecordConsolidator;
if (consolidator != null)
consolidator.EnableParseMode = true;
RecordTypeInfo typeInfo = (RecordType)reader.ReadUInt32();
if (consolidator != null)
consolidator.EnableParseMode = false;
if (typeInfo == RecordType.TES4)
return new GameHeader(reader);
else if (typeInfo.isContainer)
return new GroupHeader(reader, typeInfo, internalIDMask);
else
return new RecordHeader(reader, typeInfo, internalIDMask);
}This call leads to the execution of the overridden read method below which is where the actual problem occurs:
public override int Read(byte[] array, int offset, int count)
{
int result;
if (compressionHeaderStream == null) // condition is true when problem occurs
{
result = base.Read(array, offset, count); // ***THIS IS WHERE THE ISSUES IS ***//
if (!parseMode)
{
long equivalentBufferLocation = MemoryBufferStart + MemoryBuffer.Length;
if (Position > equivalentBufferLocation)
{
long bufferCopyBytes = Position - equivalentBufferLocation;
if (bufferCopyBytes > int.MaxValue)
throw new OverflowException();
offset += (count - (int)bufferCopyBytes);
MemoryBuffer.Write(array, offset, (int)bufferCopyBytes);
}
if (MaxBufferSize != 0)
if (MemoryBuffer.Length >= MaxBufferSize)
FlushToOutput();
}
}
else if (MemoryBuffer == null)
result = base.Read(array, offset, count);
else
result = MemoryBuffer.Read(array, offset, count);
return result;
}For some
I think you've over-engineered and should have stuck with FileStream (and an adapter pattern). You've been caught up in the fog of indirection. Inheritance has proven to be less useful than first thought; and requires a lot more thought.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
-
I think you've over-engineered and should have stuck with FileStream (and an adapter pattern). You've been caught up in the fog of indirection. Inheritance has proven to be less useful than first thought; and requires a lot more thought.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
I am not really understanding a detail of your advice here. "Fog of indirection"? I chose the simplest solution I could think of for my needs as inventing a separate class would require me sending additional parameters through a lengthy call chain involving multiple methods COMBINED with the need to use just a regular filestream to perform the same function at other times. The code worked previously. It stopped working after I reinstalled windows, Visual Studio and made some changes to add additional functionality which left that part of the code untouched.
-
I am not really understanding a detail of your advice here. "Fog of indirection"? I chose the simplest solution I could think of for my needs as inventing a separate class would require me sending additional parameters through a lengthy call chain involving multiple methods COMBINED with the need to use just a regular filestream to perform the same function at other times. The code worked previously. It stopped working after I reinstalled windows, Visual Studio and made some changes to add additional functionality which left that part of the code untouched.
-
See. Still no clarity. "Another class" is a psychological limitation, not a real one.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
Is your point to help or to show how superior your knowledge of programming is? No it isn't a psychological limitation. It is a practical one. The file stream behind the reader is used as both an indexer to keep instance data separate in a static dictionary structure AND as a way to call required additional functionality on the stream. What you suggest would require me to change a significant amount of code to pass currently unnecessary data, making it more complex that it currently is since currently I can handle separate use cases with the same sections of code (that was the reason I used the current approach). Either way the solution is complicated so why not try and fix the problem first? Unless you are saying that the problem can't be fixed using an inherited FileStream because of its "complex" nature.
-
I have made a custom filestream class that creates a copy of a file and uses the copy as the source to rewrite the file while making changes to values that match a specific criteria. Here is the basic setup:
public class MixedCompressionStreamRewriter : FileStream
It uses two modes: Parse mode reroutes the stream into a buffer to be analyzed and potentially rewritten before it gets sent to the output stream. When parse mode is false, it simply copies the base stream to the output stream. It also allows analysis of compressed data inside the stream because the source files will often have such data. Because of these use cases I had to override the Read method as such (commented so that one can trace where everything goes wrong in a debug session): The issue occurs while executing the ReadUint32 call in the code below:
public static RecordHeaderBase FromFile(BinaryReader reader, uint internalIDMask = 0xFFFFFFFF)
{
ModRecordConsolidator consolidator = reader.BaseStream as ModRecordConsolidator;
if (consolidator != null)
consolidator.EnableParseMode = true;
RecordTypeInfo typeInfo = (RecordType)reader.ReadUInt32();
if (consolidator != null)
consolidator.EnableParseMode = false;
if (typeInfo == RecordType.TES4)
return new GameHeader(reader);
else if (typeInfo.isContainer)
return new GroupHeader(reader, typeInfo, internalIDMask);
else
return new RecordHeader(reader, typeInfo, internalIDMask);
}This call leads to the execution of the overridden read method below which is where the actual problem occurs:
public override int Read(byte[] array, int offset, int count)
{
int result;
if (compressionHeaderStream == null) // condition is true when problem occurs
{
result = base.Read(array, offset, count); // ***THIS IS WHERE THE ISSUES IS ***//
if (!parseMode)
{
long equivalentBufferLocation = MemoryBufferStart + MemoryBuffer.Length;
if (Position > equivalentBufferLocation)
{
long bufferCopyBytes = Position - equivalentBufferLocation;
if (bufferCopyBytes > int.MaxValue)
throw new OverflowException();
offset += (count - (int)bufferCopyBytes);
MemoryBuffer.Write(array, offset, (int)bufferCopyBytes);
}
if (MaxBufferSize != 0)
if (MemoryBuffer.Length >= MaxBufferSize)
FlushToOutput();
}
}
else if (MemoryBuffer == null)
result = base.Read(array, offset, count);
else
result = MemoryBuffer.Read(array, offset, count);
return result;
}For some
We can't necessarily duplicate your problem from that code: we are lacking half the stuff that makes it work, such as the construction of your BinaryReader and it's FileStream. So start by creating a minimum subset app which demonstrates the problem: uses the appropriate (cut-down) classes, dumps all the extraneous code, and shows the problem in as short a code as possible (but reflecting the structure you have in your existing code). If that still shows the fault, show us the complete code so we can run it ourselves. If it doesn't, start looking at what you removed and put it back until it does ... then we can run it or it'll be fairly obvious what causes it. Me? Without running it - and I can't - I'd guess that it's working but looks like it's failing for other reasons: if the return value is
countthen it's unlikely that the read is fetching considerably more data than that (somebody would have noticed by now). So you - and we - need a minimum app to show the problem so it can be looked at closely. Sorry, but there is nothing I can do with those code fragments to prove your problem even exists!"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt AntiTwitter: @DalekDave is now a follower!
-
Is your point to help or to show how superior your knowledge of programming is? No it isn't a psychological limitation. It is a practical one. The file stream behind the reader is used as both an indexer to keep instance data separate in a static dictionary structure AND as a way to call required additional functionality on the stream. What you suggest would require me to change a significant amount of code to pass currently unnecessary data, making it more complex that it currently is since currently I can handle separate use cases with the same sections of code (that was the reason I used the current approach). Either way the solution is complicated so why not try and fix the problem first? Unless you are saying that the problem can't be fixed using an inherited FileStream because of its "complex" nature.
An "adapter" pattern lets you manipulate the filestream; instead, you want to "act" like a filestream. Since you didn't write filestream, you really have less control. You also outlined 2 distinct functions. So, filestream in fact, is now schizophrenic. Anyway, your problem seems to be you "request" "count" bytes, read, then don't bother to check "result" (actual bytes read) against "count"; and then proceed to calculate a new offset using "count" (instead of result). Not "superior"; I'm just not the coddling kind. Have a nice day.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
-
I have made a custom filestream class that creates a copy of a file and uses the copy as the source to rewrite the file while making changes to values that match a specific criteria. Here is the basic setup:
public class MixedCompressionStreamRewriter : FileStream
It uses two modes: Parse mode reroutes the stream into a buffer to be analyzed and potentially rewritten before it gets sent to the output stream. When parse mode is false, it simply copies the base stream to the output stream. It also allows analysis of compressed data inside the stream because the source files will often have such data. Because of these use cases I had to override the Read method as such (commented so that one can trace where everything goes wrong in a debug session): The issue occurs while executing the ReadUint32 call in the code below:
public static RecordHeaderBase FromFile(BinaryReader reader, uint internalIDMask = 0xFFFFFFFF)
{
ModRecordConsolidator consolidator = reader.BaseStream as ModRecordConsolidator;
if (consolidator != null)
consolidator.EnableParseMode = true;
RecordTypeInfo typeInfo = (RecordType)reader.ReadUInt32();
if (consolidator != null)
consolidator.EnableParseMode = false;
if (typeInfo == RecordType.TES4)
return new GameHeader(reader);
else if (typeInfo.isContainer)
return new GroupHeader(reader, typeInfo, internalIDMask);
else
return new RecordHeader(reader, typeInfo, internalIDMask);
}This call leads to the execution of the overridden read method below which is where the actual problem occurs:
public override int Read(byte[] array, int offset, int count)
{
int result;
if (compressionHeaderStream == null) // condition is true when problem occurs
{
result = base.Read(array, offset, count); // ***THIS IS WHERE THE ISSUES IS ***//
if (!parseMode)
{
long equivalentBufferLocation = MemoryBufferStart + MemoryBuffer.Length;
if (Position > equivalentBufferLocation)
{
long bufferCopyBytes = Position - equivalentBufferLocation;
if (bufferCopyBytes > int.MaxValue)
throw new OverflowException();
offset += (count - (int)bufferCopyBytes);
MemoryBuffer.Write(array, offset, (int)bufferCopyBytes);
}
if (MaxBufferSize != 0)
if (MemoryBuffer.Length >= MaxBufferSize)
FlushToOutput();
}
}
else if (MemoryBuffer == null)
result = base.Read(array, offset, count);
else
result = MemoryBuffer.Read(array, offset, count);
return result;
}For some
By "several thousand", do you mean 4096? The
FileStreamclass buffers the file as it reads it. By default, it uses a 4Kb buffer.FileStream Class (System.IO) | Microsoft Docs[^]:
FileStream buffers input and output for better performance.
You can pass a custom buffer size to the constructor if you want to override this behaviour. However, if you make the buffer too small, it could have a negative impact on performance.
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
An "adapter" pattern lets you manipulate the filestream; instead, you want to "act" like a filestream. Since you didn't write filestream, you really have less control. You also outlined 2 distinct functions. So, filestream in fact, is now schizophrenic. Anyway, your problem seems to be you "request" "count" bytes, read, then don't bother to check "result" (actual bytes read) against "count"; and then proceed to calculate a new offset using "count" (instead of result). Not "superior"; I'm just not the coddling kind. Have a nice day.
It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food
I don't need to be coddled lol. I just need something beyond vague references as I did not major in CS and there are a few gaps in my learning (mostly self taught). And yes... I DID check "result" which was reported as 4 bytes. So that is not the issue. The issue is that for some reason it is filling the buffer and counting it as being read when it isn't supposed to. I HAVE checked into that and confirmed it. I reported the issue on MSDN forums as something for them to look into and was told that inheriting/overloading FileStream itself can be unreliable if mixing in other unrelated functionality. I was advised to inherit Stream instead. This is similar to your original suggestion but serves my other needs as well.
-
By "several thousand", do you mean 4096? The
FileStreamclass buffers the file as it reads it. By default, it uses a 4Kb buffer.FileStream Class (System.IO) | Microsoft Docs[^]:
FileStream buffers input and output for better performance.
You can pass a custom buffer size to the constructor if you want to override this behaviour. However, if you make the buffer too small, it could have a negative impact on performance.
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
Yes. Exactly. I noticed that when I was debugging it fills the buffer and moves the Position to match the location that it WOULD be at if I was reading to the end of the buffer even though it returns that it only read 4 bytes and should have only advanced the position 4 bytes. It seems to be some behavioral bug. I posted the issue on MSDN and they said that inheriting FileStream for any complex situation such as mine is a bad idea and may result in strange behavior... I guess it did. I am now inheriting Stream instead.
-
We can't necessarily duplicate your problem from that code: we are lacking half the stuff that makes it work, such as the construction of your BinaryReader and it's FileStream. So start by creating a minimum subset app which demonstrates the problem: uses the appropriate (cut-down) classes, dumps all the extraneous code, and shows the problem in as short a code as possible (but reflecting the structure you have in your existing code). If that still shows the fault, show us the complete code so we can run it ourselves. If it doesn't, start looking at what you removed and put it back until it does ... then we can run it or it'll be fairly obvious what causes it. Me? Without running it - and I can't - I'd guess that it's working but looks like it's failing for other reasons: if the return value is
countthen it's unlikely that the read is fetching considerably more data than that (somebody would have noticed by now). So you - and we - need a minimum app to show the problem so it can be looked at closely. Sorry, but there is nothing I can do with those code fragments to prove your problem even exists!"I have no idea what I did, but I'm taking full credit for it." - ThisOldTony "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt AntiTwitter: @DalekDave is now a follower!
Yes... it is even hard for ME to duplicate as it doesn't happen in all circumstances. I have tried my class on other files successfully. The one in this particular debug was the only one giving me the issue at the time and there is nothing I can isolate. See my other recent posts if you are curious but I did solve the issue with a workaround while I change the class to inherit from Stream instead of FileStream for the final solution.
