Skip to content

BinaryWriter breaking change #107265

Closed

Description

@koszeggy

Description

When using BinaryWriter.Write(string), it attempts to optimize the performance, and when the value is short enough (and the encoding is UTF8), then the operation may bypass the Write7BitEncodedInt call.

But this is a problem if we use a derived BinaryWriter where the Write(int) method is overridden because the Write7BitEncodedInt method calls the virtual Write(int) method.

Example: we have a pair of reader-writer that dumps the written/read values while adjusts the current position. Since BinaryReader always calls the Read7BitEncodedInt method, the dumped information and offsets slowly drift away when short strings are written:

image

Reproduction Steps

  • Create a derived BinaryWriter with an overridden Write(int) method.
  • Call Write(string) with various lengths
  • Observe that the overridden Write(int) is sometimes called, sometimes not, depending on string length.

Expected behavior

BinaryWriter and BinaryReader should have a symmetric behavior. When we have a derived BinaryWriter class, I always expect the overridden Write(int) method to be called because that's how BinaryReader also works (ie. its ReadString always calls ReadInt32).

Actual behavior

BinaryWriter.Write(string) sometimes calls the implicit Write(int) method, sometimes not. See the details above.

Regression?

It works in .NET Framework, and it worked in .NET 5.0.

Known Workarounds

Duplicating the old behavior in a derived class without calling base.Write(string) (a bit painful).

Configuration

Noticed in .NET 9 Preview 7 but it turns out that we have this behavior since .NET 6.0

Other information

Possible solutions:

  • Check in the base constructor if the current type is a derived one, and if so, then initialize the _useFastUtf8 field to false.
  • Or, allow only the "semi-optimized" branch, which still calls the Write7BitEncodedInt method,

Activity

  1. dotnet-policy-service commented on Sep 2, 2024

    @dotnet-policy-service
    Contributor

    Tagging subscribers to this area: @dotnet/area-system-io
    See info in area-owners.md if you want to be subscribed.

  2. jozkee commented on Sep 3, 2024

    @jozkee
    Member

    Possibly related to #47316.

  3. added this to the 10.0.0 milestone on Sep 3, 2024
  4. added
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    and removed
    untriagedNew issue has not been triaged by the area owner
    on Sep 3, 2024
  5. added a commit that references this issue on Sep 4, 2024
    5c1a3ec
  6. added
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 4, 2024
  7. koszeggy commented on Sep 4, 2024

    @koszeggy
    ContributorAuthor

    I created a pull request.

  8. jeffhandley commented on Feb 9, 2025

    @jeffhandley
    Member

    Assigned to @adamsitnik for the necessary further triage, along with consideration of the #107369 PR.

    Thank you for your efforts and patience on this, @koszeggy.

  9. removed
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    on Mar 23, 2025
  10. modified the milestones: 10.0.0, 11.0.0 on Aug 20, 2025
  11. added a commit that references this issue on Mar 8, 2026
    9e13695
  12. locked and limited conversation to collaborators on Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area-System.IOin-prThere is an active PR which will close this issue when it is merged

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions