Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1402#discussion_r147185736
--- Diff: lib/csharp/src/Transport/TNamedPipeServerTransport.cs ---
@@ -239,40 +239,51 @@ public override void Write(byte[] buf, int off, int
len)
throw new
TTransportException(TTransportException.ExceptionType.NotOpen);
}
- if (asyncMode)
+ // if necessary, send the data in chunks
+ // there's a system limit around 0x10000 bytes that we hit
otherwise
+ // MSDN: "Pipe write operations across a network are
limited to 65,535 bytes per write. For more information regarding pipes, see
the Remarks section."
+ var nBytes = Math.Min(len, 15 * 4096); // 16 would exceed
the limit
+ while (nBytes > 0)
{
- Exception eOuter = null;
- var evt = new ManualResetEvent(false);
- stream.BeginWrite(buf, off, len, asyncResult =>
+ if (asyncMode)
{
- try
- {
- if (stream != null)
- stream.EndWrite(asyncResult);
- else
- eOuter = new
TTransportException(TTransportException.ExceptionType.Interrupted);
- }
- catch (Exception e)
- {
- if (stream != null)
- eOuter = e;
- else
- eOuter = new
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
- }
- evt.Set();
- }, null);
+ Exception eOuter = null;
+ var evt = new ManualResetEvent(false);
- evt.WaitOne();
+ stream.BeginWrite(buf, off, nBytes, asyncResult =>
+ {
+ try
+ {
+ if (stream != null)
+ stream.EndWrite(asyncResult);
+ else
+ eOuter = new
TTransportException(TTransportException.ExceptionType.Interrupted);
+ }
+ catch (Exception e)
+ {
+ if (stream != null)
+ eOuter = e;
+ else
+ eOuter = new
TTransportException(TTransportException.ExceptionType.Interrupted, e.Message);
+ }
+ evt.Set();
+ }, null);
+
+ evt.WaitOne();
+
+ if (eOuter != null)
+ throw eOuter; // rethrow exception
+ }
+ else
+ {
+ stream.Write(buf, off, nBytes);
+ }
- if (eOuter != null)
- throw eOuter; // rethrow exception
+ off += nBytes;
+ len -= nBytes;
+ nBytes = Math.Min(len, nBytes);
--- End diff --
Shouldn't this be the same calculation as the one outside the loop?
Technically it doesn't have to be, however I found this confusing to read.
---
