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

Feature request: add a delegate to be called before writing to the stream #2

Closed
IlyaBiryukov opened this issue Aug 31, 2017 · 5 comments

Comments

@IlyaBiryukov
Copy link

Something like this:

public delegate void BeforeWriteToFullDuplexStreamDelegate(FullDuplexStream stream, byte[] buffer, int offset, int count);

public class FullDuplexStream : Stream
{
       /// <summary>
       /// Gets or sets a delegate that is called just before writing to the stream,
       /// but after the caller's cancellation token has been checked.
       /// </summary>
       public BeforeWriteToFullDuplexStreamDelegate BeforeWrite { get; set; }

       /// <summary>
       /// Creates a pair of streams that can be passed to two parties
       /// to allow for interaction with each other.
       /// </summary>
       /// <returns>A pair of streams.</returns>
       public static Tuple<FullDuplexStream, FullDuplexStream> CreateStreams()
       {
           var stream1 = new FullDuplexStream();
           var stream2 = new FullDuplexStream();
           stream1.SetOtherStream(stream2);
           stream2.SetOtherStream(stream1);
           return Tuple.Create(stream1, stream2);
       }

       /// <inheritdoc />
       public override void Write(byte[] buffer, int offset, int count)
       {
           Requires.NotNull(buffer, nameof(buffer));
           Requires.Range(offset >= 0, nameof(offset));
           Requires.Range(count >= 0, nameof(count));
           Requires.Range(offset + count <= buffer.Length, nameof(count));
           this.BeforeWrite?.Invoke(this, buffer, offset, count);

           // Avoid sending an empty buffer because that is the signal of a closed stream.
           if (count > 0)
           {
               byte[] queuedBuffer = new byte[count];
               Array.Copy(buffer, offset, queuedBuffer, 0, count);
               this.other.PostMessage(new Message(queuedBuffer));
           }
    }
}
@AArnott
Copy link
Collaborator

AArnott commented Sep 1, 2017

Can you elaborate on why this is useful and can't be reached another way? This doesn't feel like natural stream behavior to have a callback like this.

@IlyaBiryukov
Copy link
Author

This is to support unit tests of streamjsonrpc. In this PR we have to copy-paste FullDuplexStream to achieve it. The idea is to be able to know when writes happen

@AArnott
Copy link
Collaborator

AArnott commented Sep 2, 2017

Could you do that by having a Read or ReadAsync call waiting for the Write operation to come in? Or is that too late?

@IlyaBiryukov
Copy link
Author

Do you mean, can we have a delegate that fires when data arrives to the read end after being written to the duplex stream? I didn't check if this gives us the same timing. In the duplex stream sources it looks like reading is async after writing, so it won't work for us.

@AArnott
Copy link
Collaborator

AArnott commented Sep 2, 2017

I don't mean a delegate. I just mean you have a reader which will unblock as soon as the writer writes to the stream. Yes, this would be slightly later timing than your proposed delegate before writing would give you.

I'm at a loss to think of how to achieve your requirement without adding a rather unnatural "feature" to this library. So I'm leaning toward encouraging you to keep your forked copy of the code for your testing purposes. Ah, the joys of open source. Is there any particular reason that you see that this would be a painful end?

AArnott added a commit that referenced this issue Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants