[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1402


---

[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185494
  
--- 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 0x1 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
--- End diff --

Why not actually use (2^16)-1 which is the limit?


---

[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185064
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 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
--- End diff --

Why not actually use `(2^16)-1` which is the limit?


---

[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[jeking3]

Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1402#discussion_r147185326
  
--- Diff: lib/csharp/src/Transport/TNamedPipeClientTransport.cs ---
@@ -88,7 +89,18 @@ public override void Write(byte[] buf, int off, int len)
 throw new 
TTransportException(TTransportException.ExceptionType.NotOpen);
 }
 
-client.Write(buf, off, len);
+// if necessary, send the data in chunks
+// there's a system limit around 0x1 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)
+{
+client.Write(buf, off, nBytes);
+
+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.


---

[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[jeking3]

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 0x1 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.


---

[GitHub] thrift pull request #1402: THRIFT-4372 Pipe write operations across a networ...

[Jens-G]

GitHub user Jens-G opened a pull request:

https://github.com/apache/thrift/pull/1402

THRIFT-4372 Pipe write operations across a network are limited to 65,…

…535 bytes per write

Client: C#
Patch: Jens Geyer

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Jens-G/thrift THRIFT-4372

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1402.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1402


commit ccfc002a480c9ed23b50674c3fd76a280750bbb0
Author: Jens Geyer 
Date:   2017-10-25T20:30:23Z

THRIFT-4372 Pipe write operations across a network are limited to 65,535 
bytes per write
Client: C#
Patch: Jens Geyer




---