Fix C# connection to correctly mark message as sent #3072
Conversation
| _writeStream.getBuffer(), | ||
| completedCallback, | ||
| this, | ||
| out messageWritten); |
There was a problem hiding this comment.
It's not clear to me why the C# startWrite has such a complicated signature compared to the C++ startWrite. In C++, we don't have this "completedSynchronously" return value.
In any case, assuming this complicated C# API is warranted, we should:
- rename the last parameter of Transceiver.startWrite. Having return value = completedSynchronously and out parameter = completed is too confusing.
- consider returning a tuple (does this work with the old C# version we use on 3.7?)
- name the "messageWritten" parameter => messageFullyWritten
- add a real doc-comment on Transceiver.startWrite
There was a problem hiding this comment.
In any case, assuming this complicated C# API is warranted, we should:
Can we do that in a separate PR?
consider returning a tuple (does this work with the old C# version we use on 3.7?)
It does not work with .NET 4.5
There was a problem hiding this comment.
Yes, this can wait until a follow-up PR.
| out messageWritten); | ||
| // If the startWrite call consumed the complete buffer, we assume the message is sent now for | ||
| // at-most-once semantics. | ||
| if (messageWritten && _sendStreams.Count > 0) |
There was a problem hiding this comment.
(Not from this PR)
Do you know when we can reach this point with _sendStreams.Count == 0?
There was a problem hiding this comment.
Seems StartWrite is some times called with a 0 buffer size just to invoke the completion callback. It is not clear why we do it that way, and cannot call it from the finishAsync.
This PR fixes C# Connection to correctly mark messages as sent, once that startWrite handles the complete buffer.
This bug was introduced in #2270, probably and oversight because of the subtle difference between C# and C++ startWrite calls.
Here to warrantee at-most once semantics we have to mark the request as sent, once that startWrite is has consumed the complete buffer.
For C++ this is the return value of
startWrite, for C# the return value is whether or not the write call completed synchronously and there is a separate out parameter that indicates whether or not the call consumed the complete buffer.This must be back ported to 3.7, but the issue doesn't affect 3.7.10.
Fixes #2722