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

Confusing Exception for NatsMemoryOwner<byte> with an empty payload #291

Closed
jeffw-wherethebitsroam opened this issue Dec 20, 2023 · 3 comments · Fixed by #295
Closed

Confusing Exception for NatsMemoryOwner<byte> with an empty payload #291

jeffw-wherethebitsroam opened this issue Dec 20, 2023 · 3 comments · Fixed by #295
Assignees
Labels
bug Something isn't working

Comments

@jeffw-wherethebitsroam
Copy link

jeffw-wherethebitsroam commented Dec 20, 2023

Observed behavior

When trying to access the memory for NatsMemoryOwner like:

var resp = await sub.Msgs.ReadAsync(cancellationToken);
using (resp.Data)
{
    var x = Type.Parser.ParseFrom(resp.Data.Memory);
    ...
}

If the reponse payload was empty, I get the Exception:

System.ObjectDisposedException: The current buffer has already been disposed

at the point where Memory is accessed. Which is confusing.

This seems to be because NatsMemoryOwner treats a null _array as having been disposed. But in NatsMsg<T>.Build, an empty payload causes the NatsMemoryOwner to be created as default, which I would assume has null for the value of _array:

        var data = payloadBuffer.Length > 0
            ? serializer.Deserialize(payloadBuffer)
            : default;

Expected behavior

When decoding an empty payload, I would expect the NatsMemoryOwner<byte> to be the equivalent of an empty array. I would only expect an ObjectDisposedException if the struct had previously been disposed.

Server and client version

Client version 2.0.2

Host environment

No response

Steps to reproduce

Decode a message with empty payload as NatsMemoryOwner<byte>.

@mtmk
Copy link
Collaborator

mtmk commented Dec 21, 2023

I guess the workaround for now is to check the length

var resp = await sub.Msgs.ReadAsync(cancellationToken);
if (resp.Data.Length > 0)
{
  using (resp.Data)
  {
      var x = Type.Parser.ParseFrom(resp.Data.Memory);
      ...
  }
}

does that work for you?

@jeffw-wherethebitsroam
Copy link
Author

Yes, that it what I ended up doing.

But that was after way too much time spent trying to work out how the NatsMemoryOwner had been disposed before I used it! The only way I worked it out in the end was by going through the Nats client source code. I'm just trying to save others from my pain :)

@mtmk
Copy link
Collaborator

mtmk commented Dec 21, 2023

Yes, that it what I ended up doing.

But that was after way too much time spent trying to work out how the NatsMemoryOwner had been disposed before I used it! The only way I worked it out in the end was by going through the Nats client source code. I'm just trying to save others from my pain :)

No you're absolutely right. I'm looking into it. just wanted to note a workaround now before we released a fix.

thank you for the detailed reports btw. very helpful 👍

@mtmk mtmk self-assigned this Dec 21, 2023
@mtmk mtmk linked a pull request Dec 21, 2023 that will close this issue
@mtmk mtmk added the bug Something isn't working label Dec 21, 2023
@mtmk mtmk closed this as completed in #295 Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants