Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization: PooledArrayBufferWriter - AsReadOnlySequence does not return full output #8503

Closed
Romanx opened this issue Jun 22, 2023 · 2 comments · Fixed by #8504
Closed

Comments

@Romanx
Copy link
Contributor

Romanx commented Jun 22, 2023

I ran into this problem and have reduced it down to the following repro.

It seems to be that the SequenceSegment Blocks cache for pooling on dispose of the first serialization session manages to get the same segment added twice and since it's a ConcurrentBag the set is not distinct.

This means that the second serialization attempt gets the pooled blocks and the first two requests get the same SequenceSegment which causes an odd linked list and data to be lost

using Microsoft.Extensions.DependencyInjection;
using Orleans.Serialization;
using Orleans.Serialization.Buffers;
using Orleans.Serialization.Buffers.Adaptors;
using Orleans.Serialization.Session;

namespace OrleansSerializerBug;

internal class Program
{
    static void Main(string[] args)
    {
        var serviceProvider = new ServiceCollection()
            .AddSerializer()
            .BuildServiceProvider();

        var pool = serviceProvider.GetRequiredService<SerializerSessionPool>();
        var serializer = serviceProvider.GetRequiredService<Serializer>();

        var obj = LargeObject.BuildRandom();

        // First Works Fine
        SerializeObject(pool, serializer, obj);

        // Second Fails
        SerializeObject(pool, serializer, obj);

        static void SerializeObject(SerializerSessionPool pool, Serializer serializer, LargeObject obj)
        {
            Writer<PooledArrayBufferWriter> writer = default;
            try
            {
                writer = Writer.CreatePooled(pool.GetSession());
                serializer.Serialize(obj, ref writer);

                var sequence = writer.Output.AsReadOnlySequence();
                if (sequence.Length != writer.Output.Length)
                {
                    Console.WriteLine($"""
                        Output sequence has length "{sequence.Length}" that does not match writer output length "{writer.Output.Length}"
                        """);
                }
            }
            finally
            {
                writer.Dispose();
            }
        }
    }
}

[GenerateSerializer]
public readonly record struct LargeObject(
[property: Id(0)] Guid Id,
[property: Id(1)] (Guid, Guid)[] Values)
{
    public static LargeObject BuildRandom()
    {
        var id = Guid.NewGuid();
        var values = new (Guid, Guid)[256];

        for (var i = 0; i < values.Length; i++)
        {
            values[i] = (Guid.NewGuid(), Guid.NewGuid());
        }

        return new(id, values);
    }
}
@ghost ghost added the Needs: triage 🔍 label Jun 22, 2023
@ReubenBond
Copy link
Member

Which version of Orleans is this? I think this was likely fixed in #8453, but we ought to include this test

@Romanx
Copy link
Contributor Author

Romanx commented Jun 22, 2023

Sorry missed that on the original post. This is using latest at the time of posting: 7.1.2

ReubenBond added a commit to ReubenBond/orleans that referenced this issue Jun 22, 2023
ReubenBond added a commit that referenced this issue Jun 22, 2023
@ghost ghost added the Status: Fixed label Jun 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants