[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122799#comment-16122799
 ] 

ASF GitHub Bot commented on THRIFT-3821:


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

https://github.com/apache/thrift/pull/1326#discussion_r132613779
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Basically, if we only go through powers of 2 when resizing (which looks to 
be the case), the buffer can only grow to 2GB since the maximum value of 
`uint32_t` is one short of 4GB.  


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread asuhan
Github user asuhan commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132613779
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Basically, if we only go through powers of 2 when resizing (which looks to 
be the case), the buffer can only grow to 2GB since the maximum value of 
`uint32_t` is one short of 4GB.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread asuhan
Github user asuhan commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132612556
  
--- Diff: lib/cpp/test/TMemoryBufferTest.cpp ---
@@ -117,4 +117,17 @@ BOOST_AUTO_TEST_CASE(test_exceptions) {
   BOOST_CHECK_NO_THROW(buf2.write((const uint8_t*)"bar", 3));
 }
 
+#ifndef _WIN32
+// We can't allocate 1 GB of memory in 32-bit environments.
--- End diff --

We'd have to write in a loop to trigger buffer resizing if I understand 
correctly. It'd save the vector allocation in the test, but `TMemoryBuffer` 
would still grow. I agree a 50% reduction would be better than nothing, for 
sure.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122776#comment-16122776
 ] 

ASF GitHub Bot commented on THRIFT-3821:


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

https://github.com/apache/thrift/pull/1326#discussion_r132612265
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

We check it post-resize and it fails if we go past 4 GB, therefore 
pre-resize it must be at most 2GB. Checking against `int32_t` would be overly 
conservative; we mainly care about avoiding the arithmetic overflow. Maybe the 
error message could be improved? `"Internal buffer size was already past 2GB 
when we attempted to resize"` would be a more precise description, but I didn't 
want to make it overly verbose / obscure.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread asuhan
Github user asuhan commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132612265
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

We check it post-resize and it fails if we go past 4 GB, therefore 
pre-resize it must be at most 2GB. Checking against `int32_t` would be overly 
conservative; we mainly care about avoiding the arithmetic overflow. Maybe the 
error message could be improved? `"Internal buffer size was already past 2GB 
when we attempted to resize"` would be a more precise description, but I didn't 
want to make it overly verbose / obscure.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] thrift issue #1279: THRIFT-4212: Fix flush on ssl socket thrift

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1279
  
When THRIFT-4211 is clean in CI and merged, you will need to rebase this 
since it includes THRIFT-4211, and then we can get it into CI for a build then 
merge.  Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-4212) c_glib flush tries to close SSL even if socket is invalid

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122775#comment-16122775
 ] 

ASF GitHub Bot commented on THRIFT-4212:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1279
  
When THRIFT-4211 is clean in CI and merged, you will need to rebase this 
since it includes THRIFT-4211, and then we can get it into CI for a build then 
merge.  Thanks.


> c_glib flush tries to close SSL even if socket is invalid
> -
>
> Key: THRIFT-4212
> URL: https://issues.apache.org/jira/browse/THRIFT-4212
> Project: Thrift
>  Issue Type: Bug
>  Components: C glib - Library
>Affects Versions: 1.0
>Reporter: Gonzalo Aguilar
>Assignee: Gonzalo Aguilar
> Fix For: 1.0
>
>
> It seems that SSL is trying to get info from socket even when socket is 
> already invalid, making the app to crash.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4211) Fix GError glib management under Thrift

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122773#comment-16122773
 ] 

ASF GitHub Bot commented on THRIFT-4211:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1278
  
Would you be able to rebase against master and push so there is one commit 
(just like there is now), but more importantly it will kick off a new CI build 
for us.  Thanks.


> Fix GError glib management under Thrift
> ---
>
> Key: THRIFT-4211
> URL: https://issues.apache.org/jira/browse/THRIFT-4211
> Project: Thrift
>  Issue Type: Bug
>  Components: C glib - Library
>Affects Versions: 1.0
>Reporter: Gonzalo Aguilar
>Assignee: Gonzalo Aguilar
> Fix For: 1.0
>
>
> It seems that current Gerror management done in thrift is not quite ok and 
> causes the library to fail. 
> This issue tracks and fixes all problems found during testing.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1278: THRIFT-4211: Fix logging in thrift library

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1278
  
Would you be able to rebase against master and push so there is one commit 
(just like there is now), but more importantly it will kick off a new CI build 
for us.  Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] thrift pull request #1265: THRIFT-3963 Thrift.cabal filename does not match ...

2017-08-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-3963) Thrift.cabal filename does not match module name

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122771#comment-16122771
 ] 

ASF GitHub Bot commented on THRIFT-3963:


Github user asfgit closed the pull request at:

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


> Thrift.cabal filename does not match module name
> 
>
> Key: THRIFT-3963
> URL: https://issues.apache.org/jira/browse/THRIFT-3963
> Project: Thrift
>  Issue Type: Bug
>  Components: Haskell - Library
>Affects Versions: 0.10.0
> Environment: Haskell Stack with LTS 7.7, on Ubuntu
>Reporter: Isaac Sheff
>Assignee: James E. King, III
>  Labels: github-import
> Fix For: 0.11.0
>
>   Original Estimate: 20m
>  Remaining Estimate: 20m
>
> Some haskell environments, including stack (http://haskellstack.org/) require 
> that the cabal file name match the module name (case sensitive). This means 
> that it is difficult to import the (git source repository) thrift library 
> into stack projects (https://github.com/commercialhaskell/stack/issues/317). 
> The fix is easy: change the name "Thrift.cabal" to "thrift.cabal."



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Resolved] (THRIFT-3963) Thrift.cabal filename does not match module name

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-3963?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III resolved THRIFT-3963.

   Resolution: Fixed
Fix Version/s: 0.11.0

> Thrift.cabal filename does not match module name
> 
>
> Key: THRIFT-3963
> URL: https://issues.apache.org/jira/browse/THRIFT-3963
> Project: Thrift
>  Issue Type: Bug
>  Components: Haskell - Library
>Affects Versions: 0.10.0
> Environment: Haskell Stack with LTS 7.7, on Ubuntu
>Reporter: Isaac Sheff
>Assignee: James E. King, III
>  Labels: github-import
> Fix For: 0.11.0
>
>   Original Estimate: 20m
>  Remaining Estimate: 20m
>
> Some haskell environments, including stack (http://haskellstack.org/) require 
> that the cabal file name match the module name (case sensitive). This means 
> that it is difficult to import the (git source repository) thrift library 
> into stack projects (https://github.com/commercialhaskell/stack/issues/317). 
> The fix is easy: change the name "Thrift.cabal" to "thrift.cabal."



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-3963) Thrift.cabal filename does not match module name

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-3963?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III updated THRIFT-3963:
---
Affects Version/s: (was: 1.0)
   0.10.0

> Thrift.cabal filename does not match module name
> 
>
> Key: THRIFT-3963
> URL: https://issues.apache.org/jira/browse/THRIFT-3963
> Project: Thrift
>  Issue Type: Bug
>  Components: Haskell - Library
>Affects Versions: 0.10.0
> Environment: Haskell Stack with LTS 7.7, on Ubuntu
>Reporter: Isaac Sheff
>  Labels: github-import
>   Original Estimate: 20m
>  Remaining Estimate: 20m
>
> Some haskell environments, including stack (http://haskellstack.org/) require 
> that the cabal file name match the module name (case sensitive). This means 
> that it is difficult to import the (git source repository) thrift library 
> into stack projects (https://github.com/commercialhaskell/stack/issues/317). 
> The fix is easy: change the name "Thrift.cabal" to "thrift.cabal."



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Assigned] (THRIFT-3963) Thrift.cabal filename does not match module name

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-3963?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III reassigned THRIFT-3963:
--

Assignee: James E. King, III

> Thrift.cabal filename does not match module name
> 
>
> Key: THRIFT-3963
> URL: https://issues.apache.org/jira/browse/THRIFT-3963
> Project: Thrift
>  Issue Type: Bug
>  Components: Haskell - Library
>Affects Versions: 0.10.0
> Environment: Haskell Stack with LTS 7.7, on Ubuntu
>Reporter: Isaac Sheff
>Assignee: James E. King, III
>  Labels: github-import
>   Original Estimate: 20m
>  Remaining Estimate: 20m
>
> Some haskell environments, including stack (http://haskellstack.org/) require 
> that the cabal file name match the module name (case sensitive). This means 
> that it is difficult to import the (git source repository) thrift library 
> into stack projects (https://github.com/commercialhaskell/stack/issues/317). 
> The fix is easy: change the name "Thrift.cabal" to "thrift.cabal."



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4248) Compile fails - strncpy, memcmp, memset not declared in src/thrift/transport/TSSLSocket.cpp

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122765#comment-16122765
 ] 

ASF GitHub Bot commented on THRIFT-4248:


Github user asfgit closed the pull request at:

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


> Compile fails - strncpy, memcmp, memset not declared in 
> src/thrift/transport/TSSLSocket.cpp
> ---
>
> Key: THRIFT-4248
> URL: https://issues.apache.org/jira/browse/THRIFT-4248
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
>Reporter: Josip Sokcevic
>Assignee: James E. King, III
> Fix For: 0.11.0
>
> Attachments: 0001-THRIFT-4248-Import-cstring-in-TSSLSocket.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1307: THRIFT-4248: Import cstring in TSSLSocket

2017-08-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Assigned] (THRIFT-4248) Compile fails - strncpy, memcmp, memset not declared in src/thrift/transport/TSSLSocket.cpp

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-4248?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III reassigned THRIFT-4248:
--

Assignee: James E. King, III

> Compile fails - strncpy, memcmp, memset not declared in 
> src/thrift/transport/TSSLSocket.cpp
> ---
>
> Key: THRIFT-4248
> URL: https://issues.apache.org/jira/browse/THRIFT-4248
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
>Reporter: Josip Sokcevic
>Assignee: James E. King, III
> Fix For: 0.11.0
>
> Attachments: 0001-THRIFT-4248-Import-cstring-in-TSSLSocket.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4248) Compile fails - strncpy, memcmp, memset not declared in src/thrift/transport/TSSLSocket.cpp

2017-08-10 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122758#comment-16122758
 ] 

James E. King, III commented on THRIFT-4248:


Compile fails - on what platform / environment?  Not in CI and not on any of my 
machines?

> Compile fails - strncpy, memcmp, memset not declared in 
> src/thrift/transport/TSSLSocket.cpp
> ---
>
> Key: THRIFT-4248
> URL: https://issues.apache.org/jira/browse/THRIFT-4248
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
>Reporter: Josip Sokcevic
> Attachments: 0001-THRIFT-4248-Import-cstring-in-TSSLSocket.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122757#comment-16122757
 ] 

ASF GitHub Bot commented on THRIFT-3821:


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

https://github.com/apache/thrift/pull/1326#discussion_r132611000
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Shouldn't this be int32_t instead of uint32_t, if we want to test against 
2GB?  Sorry I didn't catch this before.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122756#comment-16122756
 ] 

ASF GitHub Bot commented on THRIFT-3821:


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

https://github.com/apache/thrift/pull/1326#discussion_r132611133
  
--- Diff: lib/cpp/test/TMemoryBufferTest.cpp ---
@@ -117,4 +117,17 @@ BOOST_AUTO_TEST_CASE(test_exceptions) {
   BOOST_CHECK_NO_THROW(buf2.write((const uint8_t*)"bar", 3));
 }
 
+#ifndef _WIN32
+// We can't allocate 1 GB of memory in 32-bit environments.
--- End diff --

This makes me wonder if the TBufferTransport should have a size limit that 
is configurable, with a default of INT32_MAX, and then the test can make a 
smaller one like 4KB, and write 4KB and then one byte more, instead of using up 
2GB of memory.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132611000
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,13 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
--- End diff --

Shouldn't this be int32_t instead of uint32_t, if we want to test against 
2GB?  Sorry I didn't catch this before.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132611133
  
--- Diff: lib/cpp/test/TMemoryBufferTest.cpp ---
@@ -117,4 +117,17 @@ BOOST_AUTO_TEST_CASE(test_exceptions) {
   BOOST_CHECK_NO_THROW(buf2.write((const uint8_t*)"bar", 3));
 }
 
+#ifndef _WIN32
+// We can't allocate 1 GB of memory in 32-bit environments.
--- End diff --

This makes me wonder if the TBufferTransport should have a size limit that 
is configurable, with a default of INT32_MAX, and then the test can make a 
smaller one like 4KB, and write 4KB and then one byte more, instead of using up 
2GB of memory.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122696#comment-16122696
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on the issue:

https://github.com/apache/thrift/pull/1326
  
I've disabled the test for Windows since allocating 1GB on 32-bit is likely 
to fail with `bad_alloc`.

Test failure looks unrelated, also got a clean run before: 
https://travis-ci.org/apache/thrift/jobs/263298365.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122386#comment-16122386
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user asuhan commented on the issue:

https://github.com/apache/thrift/pull/1326
  
Thanks for the review, I'll address both. It's my preference to squash the 
commits as well, sounds like I've misinterpreted 
https://thrift.apache.org/docs/HowToContribute (it says "When bugfixing: add 
test that will isolate bug before applying change that fixes it").


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-4246) Sequence number mismatch on multiplexed clients

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4246?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122274#comment-16122274
 ] 

ASF GitHub Bot commented on THRIFT-4246:


Github user boivie commented on the issue:

https://github.com/apache/thrift/pull/1322
  
Thanks James. I just rebased it, but travis and AppVeyor fails from 
unrelated reasons. Can they be retriggered?


> Sequence number mismatch on multiplexed clients
> ---
>
> Key: THRIFT-4246
> URL: https://issues.apache.org/jira/browse/THRIFT-4246
> Project: Thrift
>  Issue Type: Bug
>  Components: Node.js - Library
>Affects Versions: 0.10.0
>Reporter: Victor Boivie
>Priority: Critical
> Attachments: 
> 0001-THRIFT-4246-Multiplexed-clients-sequence-id-fix.patch
>
>
> When performing calls using a multiplexed client and when having multiple 
> connections and clients, the wrong sequence numbers are used which will often 
> result in responses not being able to be delivered to the client. This is 
> because every connection will make the client class (not instance) use the 
> latest created multiplexer class to generate sequence numbers. The following 
> example shows the problem:
> {code:javascript}
> const thrift = require('thrift');
> const AlphaService = require('./gen-nodejs/AlphaService');
> const BetaService = require('./gen-nodejs/BetaService');
> let connection1 = thrift.createConnection('localhost', 9091, {
>   transport: thrift.TFrameTransport,
>   protocol: thrift.TCompactProtocol,
> });
> let multiplexer1 = new thrift.Multiplexer();
> let alpha1 = multiplexer1.createClient('alpha', AlphaService, connection1);
> let beta1 = multiplexer1.createClient('beta', BetaService, connection1);
> alpha1.echoAlpha("hello")
> let connection2 = thrift.createConnection('localhost', 9091, {
>   transport: thrift.TFrameTransport,
>   protocol: thrift.TCompactProtocol,
> });
> let multiplexer2 = new thrift.Multiplexer();
> let alpha2 = multiplexer2.createClient('alpha', AlphaService, connection2);
> let beta2 = multiplexer2.createClient('beta', BetaService, connection2);
> beta1.echoBeta("Hi")
> beta2.echoBeta("hello")
> console.log("alpha1 seqId", alpha1._seqid)
> console.log("beta1 seqId", beta1._seqid)
> console.log("beta2 seqId", beta2._seqid)
> console.log("multiplexer1 latest", multiplexer1.seqid)
> console.log("multiplexer2 latest", multiplexer2.seqid)
> console.log("connection1 mapping", connection1.seqId2Service)
> console.log("connection2 mapping", connection2.seqId2Service)
> {code}
> Result:
> {noformat}
> alpha1 seqId 1
> beta1 seqId 1
> beta2 seqId 2
> multiplexer1 latest 1
> multiplexer2 latest 2
> connection1 mapping { '1': 'beta' }
> connection2 mapping { '2': 'beta' }
> {noformat}
> Connection1 should have mapping 1 -> alpha, 2-> beta



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1322: THRIFT-4246 Multiplexed clients sequence id fix

2017-08-10 Thread boivie
Github user boivie commented on the issue:

https://github.com/apache/thrift/pull/1322
  
Thanks James. I just rebased it, but travis and AppVeyor fails from 
unrelated reasons. Can they be retriggered?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-4274) Python feature tests for SSL/TLS failing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122187#comment-16122187
 ] 

ASF GitHub Bot commented on THRIFT-4274:


Github user asfgit closed the pull request at:

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


> Python feature tests for SSL/TLS failing
> 
>
> Key: THRIFT-4274
> URL: https://issues.apache.org/jira/browse/THRIFT-4274
> Project: Thrift
>  Issue Type: Bug
>  Components: Python - Library
> Environment: Arch Linux, Python 3.6.2, OpenSSL 1.1.0.f
>Reporter: Håkon Hitland
>Assignee: James E. King, III
>Priority: Minor
> Fix For: 0.11.0
>
>
> {{make cross}} fails on the Python feature tests {{py-nosslv3}} and 
> {{py-tls}}, as the test server exits with a stack trace like
> {noformat}
> Traceback (most recent call last):
>   File "/thrift/test/py/TestServer.py", line 315, in 
> sys.exit(main(options))
>   File "/thrift/test/py/TestServer.py", line 268, in main
> server.serve()
>   File "/thrift/lib/py/build/lib.linux-x86_64-3.6/thrift/server/TServer.py", 
> line 76, in serve
> client = self.serverTransport.accept()
>   File 
> "/thrift/lib/py/build/lib.linux-x86_64-3.6/thrift/transport/TSSLSocket.py", 
> line 370, in accept
> client = self._wrap_socket(plain_client)
>   File 
> "/thrift/lib/py/build/lib.linux-x86_64-3.6/thrift/transport/TSSLSocket.py", 
> line 184, in _wrap_socket
> server_hostname=self._server_hostname)
>   File "/usr/lib/python3.6/ssl.py", line 401, in wrap_socket
> _context=self, _session=session)
>   File "/usr/lib/python3.6/ssl.py", line 808, in __init__
> self.do_handshake()
>   File "/usr/lib/python3.6/ssl.py", line 1061, in do_handshake
> self._sslobj.do_handshake()
>   File "/usr/lib/python3.6/ssl.py", line 683, in do_handshake
> self._sslobj.do_handshake()
> OSError: [Errno 0] Error
> {noformat}
> This seems to be caused by {{ensure_socket_open}} in 
> {{test/crossrunner/run.py}}, which will make a connection and immediately 
> close the socket without completing the handshake.
> On the server side this causes an {{OSError}} to be thrown from 
> {{SSLContext.wrap_socket()}}, which is not caught in 
> {{TSSLServerSocket.accept()}}.
> This works on older installations, so presumably either Python or OpenSSL has 
> changed to cause an {{OSError}} instead of a {{SSLError}}.
> Catching {{OSError}} in {{accept()}} and handling it the same as {{SSLError}} 
> looks like a reasonable fix.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1321: THRIFT-4274: Catch OSError in TSSLServerSocket.ac...

2017-08-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122170#comment-16122170
 ] 

ASF GitHub Bot commented on THRIFT-3821:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1326
  
Please rebase (and squash to a single commit, if you can) and push here, so 
the build system can build the change and we can verify it before merging it.  
Thanks.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-3775) 0.10.0 release candidate

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122171#comment-16122171
 ] 

ASF GitHub Bot commented on THRIFT-3775:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1323
  
@jfarrell this needs to be closed.  It is invalid.


> 0.10.0 release candidate
> 
>
> Key: THRIFT-3775
> URL: https://issues.apache.org/jira/browse/THRIFT-3775
> Project: Thrift
>  Issue Type: Bug
>  Components: Build Process
>Reporter: Jake Farrell
>Assignee: Jake Farrell
> Fix For: 0.10.0
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1323: THRIFT-3775: 0.10.0 release

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1323
  
@jfarrell this needs to be closed.  It is invalid.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-3821) TMemoryBuffer buffer may overflow when resizing

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122168#comment-16122168
 ] 

ASF GitHub Bot commented on THRIFT-3821:


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

https://github.com/apache/thrift/pull/1326#discussion_r132545680
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,12 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
+  throw TTransportException("Internal buffer size exceeded 2GB");
--- End diff --

I would prefer to see a TTransportException with an error type BAD_ARGS 
here, however it looks like error types are not being used elsewhere in this 
module.


> TMemoryBuffer buffer may overflow when resizing
> ---
>
> Key: THRIFT-3821
> URL: https://issues.apache.org/jira/browse/THRIFT-3821
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.9.3
>Reporter: Huaisi Xu
>
> In ensurecanwrite():
> {code}
>   uint32_t new_size = bufferSize_;
>   while (len > avail) {
> new_size = new_size > 0 ? new_size * 2 : 1;
> avail = available_write() + (new_size - bufferSize_);
>   }
>   // Allocate into a new pointer so we don't bork ours if it fails.
>   uint8_t* new_buffer = static_cast(std::realloc(buffer_, 
> new_size));
>   if (new_buffer == NULL) {
> throw std::bad_alloc();
>   }
>   rBase_ = new_buffer + (rBase_ - buffer_);
>   rBound_ = new_buffer + (rBound_ - buffer_);
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
>   buffer_ = new_buffer;
>   bufferSize_ = new_size;
> {code}
> If old bufferSize_ is lager than 2gb, then calculating new size will overflow.
> i.e. if bufferSize_ = 3355443200, then new buffer size will be 2415919104, 
> which is less than old size.
> However, 
> {code}
> avail = available_write() + (new_size - bufferSize_) 
> {code}
> overflows again, so we will end up with an shrinked buffer.
> What is worse is that after
> {code}
>   wBase_ = new_buffer + (wBase_ - buffer_);
>   wBound_ = new_buffer + new_size;
> {code}
> wBase_ stays the same, but wBound_ becomes lower than it. What happens next 
> is that   uint32_t avail = available_write() may overflow every time 
> subsequently. and thrift writes to unknown memory.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1326: THRIFT-3821

2017-08-10 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1326#discussion_r132545680
  
--- Diff: lib/cpp/src/thrift/transport/TBufferTransports.cpp ---
@@ -361,9 +361,12 @@ void TMemoryBuffer::ensureCanWrite(uint32_t len) {
   }
 
   // Grow the buffer as necessary.
-  uint32_t new_size = bufferSize_;
+  uint64_t new_size = bufferSize_;
   while (len > avail) {
 new_size = new_size > 0 ? new_size * 2 : 1;
+if (new_size > std::numeric_limits::max()) {
+  throw TTransportException("Internal buffer size exceeded 2GB");
--- End diff --

I would prefer to see a TTransportException with an error type BAD_ARGS 
here, however it looks like error types are not being used elsewhere in this 
module.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] thrift issue #1327: make thrift error:class ‘apache::thrift::transport::TH...

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1327
  
@jfarrell this pull request is bogus and this topic should be closed.

I reproduced your issue with THeaderTransport under CentOS 6.9 and the same 
installation instructions.  The construction arguments need to pass the 
complete template in this case:

: TVirtualTransport(inTransport)

instead of

: TVirtualTransport(inTransport)

That should get you past the issue.
It appears none of the more recent compilers require this to function 
properly, so we're probably not going to patch for it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread Mario Emmenlauer (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16122089#comment-16122089
 ] 

Mario Emmenlauer commented on THRIFT-2221:
--

Thanks again [~jking] , super great work and its highly appreciated! I'm very 
much looking forward to this release!

> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler, C++ - Library
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
> Fix For: 0.11.0
>
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)
> Solution summary (so you don't need to wade through the comments):
> A new header, , creates the namespace apache::thrift::stdcxx 
> and aliases the proper std:: or boost:: constructs depending on the 
> capabilities of the environment and whether any build flags are used to force 
> an override of this behavior.  All code throughout the project now uses 
> stdcxx:: for all smart_ptr and functional (bind, function, placeholders, _1, 
> _2, ...) constructs.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-3978) Thrift C++ runtime uses assert to prevent overflows, checks sanity only in debug builds

2017-08-10 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16121982#comment-16121982
 ] 

James E. King, III commented on THRIFT-3978:


The following still need to be cleaned up before we can consider this story 
complete:
{noformat}
jking@dvm:~/thrift/github/thrift$ find lib/cpp/src -type f -name '*.cpp' -exec 
grep -l 'assert' {} \;
lib/cpp/src/thrift/VirtualProfiling.cpp
lib/cpp/src/thrift/windows/OverlappedSubmissionThread.cpp
lib/cpp/src/thrift/windows/TWinsockSingleton.cpp
lib/cpp/src/thrift/server/TNonblockingServer.cpp
lib/cpp/src/thrift/protocol/TBase64Utils.cpp
lib/cpp/src/thrift/protocol/TDebugProtocol.cpp
lib/cpp/src/thrift/protocol/THeaderProtocol.cpp
lib/cpp/src/thrift/protocol/TJSONProtocol.cpp
lib/cpp/src/thrift/concurrency/Monitor.cpp
lib/cpp/src/thrift/concurrency/BoostMonitor.cpp
lib/cpp/src/thrift/concurrency/TimerManager.cpp
lib/cpp/src/thrift/concurrency/StdThreadFactory.cpp
lib/cpp/src/thrift/concurrency/BoostThreadFactory.cpp
lib/cpp/src/thrift/concurrency/Util.cpp
lib/cpp/src/thrift/concurrency/StdMutex.cpp
lib/cpp/src/thrift/concurrency/Mutex.cpp
lib/cpp/src/thrift/concurrency/StdMonitor.cpp
lib/cpp/src/thrift/concurrency/BoostMutex.cpp
lib/cpp/src/thrift/concurrency/PosixThreadFactory.cpp
lib/cpp/src/thrift/transport/TFileTransport.cpp
lib/cpp/src/thrift/transport/TBufferTransports.cpp
lib/cpp/src/thrift/transport/TSSLSocket.cpp
lib/cpp/src/thrift/transport/TZlibTransport.cpp
lib/cpp/src/thrift/transport/TPipe.cpp
lib/cpp/src/thrift/async/TEvhttpClientChannel.cpp
{noformat}

> Thrift C++ runtime uses assert to prevent overflows, checks sanity only in 
> debug builds
> ---
>
> Key: THRIFT-3978
> URL: https://issues.apache.org/jira/browse/THRIFT-3978
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
> Environment: All
>Reporter: James E. King, III
>Assignee: James E. King, III
>  Labels: security
>
> Currently there is widespread use of assert in the thrift C++ runtime 
> library.  Some of the more disturbing cases are security related, for example 
> checking header sizes.  I recommend we eliminate assertions that are only 
> checked in debug mode, and instead throw the appropriate exception, usually a 
> TTransportException with CORRUPTED_DATA as the reason.  If we're going to 
> check for an overflow or a buffer overrun, we should do so in debug and 
> release modes.  Further, assertions are not easily tested whereas exceptions 
> are.
> In THRIFT-3873 apache::thrift::transport::safe_numeric_cast was added, so I 
> also suggest changing static_cast to safe_numeric_cast where appropriate 
> throughout the transport code to catch any overflow errors.
> Another location where assert is used liberally is inside the posix Mutex 
> implementation.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III updated THRIFT-2221:
---
Description: 
Most modern compilers now have full support for std::shared_ptr when enable 
with c++11 flags. It would be nice to have the option to generate code that 
uses this instead of boost::shared_ptr. This would enable us to remove another 
boost dependency, on the road to a dependency-free thrift library :)

Solution summary (so you don't need to wade through the comments):

A new header, , creates the namespace apache::thrift::stdcxx 
and aliases the proper std:: or boost:: constructs depending on the 
capabilities of the environment and whether any build flags are used to force 
an override of this behavior.  All code throughout the project now uses 
stdcxx:: for all smart_ptr and functional (bind, function, placeholders, _1, 
_2, ...) constructs.

  was:Most modern compilers now have full support for std::shared_ptr when 
enable with c++11 flags. It would be nice to have the option to generate code 
that uses this instead of boost::shared_ptr. This would enable us to remove 
another boost dependency, on the road to a dependency-free thrift library :)


> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler, C++ - Library
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
> Fix For: 0.11.0
>
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)
> Solution summary (so you don't need to wade through the comments):
> A new header, , creates the namespace apache::thrift::stdcxx 
> and aliases the proper std:: or boost:: constructs depending on the 
> capabilities of the environment and whether any build flags are used to force 
> an override of this behavior.  All code throughout the project now uses 
> stdcxx:: for all smart_ptr and functional (bind, function, placeholders, _1, 
> _2, ...) constructs.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III updated THRIFT-2221:
---
Fix Version/s: 0.11.0
  Component/s: C++ - Library

> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler, C++ - Library
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
> Fix For: 0.11.0
>
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Resolved] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread James E. King, III (JIRA)

 [ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

James E. King, III resolved THRIFT-2221.

Resolution: Fixed

> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler, C++ - Library
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
> Fix For: 0.11.0
>
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread James E. King, III (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16121953#comment-16121953
 ] 

James E. King, III commented on THRIFT-2221:


Okay that one was fun... :)

> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16121952#comment-16121952
 ] 

ASF GitHub Bot commented on THRIFT-2221:


Github user asfgit closed the pull request at:

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


> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift pull request #1328: THRIFT-2221: detect C++11 and use std namespace f...

2017-08-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16121937#comment-16121937
 ] 

ASF GitHub Bot commented on THRIFT-2221:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1328
  
The lone build failure is a "D" language/test issue that appears 
sporadically.  All other builds passed.   As such, I am going to merge this in 
given it was already reviewed, and the follow-on changes were fairly trivial.


> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1328: THRIFT-2221: detect C++11 and use std namespace for memo...

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1328
  
The lone build failure is a "D" language/test issue that appears 
sporadically.  All other builds passed.   As such, I am going to merge this in 
given it was already reviewed, and the follow-on changes were fairly trivial.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Commented] (THRIFT-2221) Generate c++ code with std::shared_ptr instead of boost::shared_ptr.

2017-08-10 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-2221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16121665#comment-16121665
 ] 

ASF GitHub Bot commented on THRIFT-2221:


Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1328
  
I added a TPipe test because I changed one line of code in TPipe to work 
around a build issue, and I didn't see a TTransportTest test for TPipe.


> Generate c++ code with std::shared_ptr instead of boost::shared_ptr.
> 
>
> Key: THRIFT-2221
> URL: https://issues.apache.org/jira/browse/THRIFT-2221
> Project: Thrift
>  Issue Type: Improvement
>  Components: C++ - Compiler
>Affects Versions: 0.9.1
> Environment: C++11 compilers with std::shared_ptr support
>Reporter: Chris Stylianou
>Assignee: James E. King, III
>  Labels: c++11, compiler, thrift
>
> Most modern compilers now have full support for std::shared_ptr when enable 
> with c++11 flags. It would be nice to have the option to generate code that 
> uses this instead of boost::shared_ptr. This would enable us to remove 
> another boost dependency, on the road to a dependency-free thrift library :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[GitHub] thrift issue #1328: THRIFT-2221: detect C++11 and use std namespace for memo...

2017-08-10 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1328
  
I added a TPipe test because I changed one line of code in TPipe to work 
around a build issue, and I didn't see a TTransportTest test for TPipe.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Updated] (THRIFT-4283) TNamedPipeServer race condition in interrupt

2017-08-10 Thread JIRA

 [ 
https://issues.apache.org/jira/browse/THRIFT-4283?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jean-Noël Goor updated THRIFT-4283:
---
Labels: c++  (was: )

> TNamedPipeServer race condition in interrupt
> 
>
> Key: THRIFT-4283
> URL: https://issues.apache.org/jira/browse/THRIFT-4283
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
> Environment: Windows
>Reporter: Jean-Noël Goor
>  Labels: c++
> Attachments: thrift-4283-TNamedPipeServer-race-condition.patch
>
>
> In TNamedPipeServer, there exists a race condition between the 'serve' thread 
> and another thread that would attempt to interrupt it.
> While the 'serve' thread is waiting for clients to connect, it is blocked on 
> a GetOverlappedResult call.
> To stop the server, another thread calls the interrupt function that submits 
> a CANCELIO operation to the TOverlappedSubmissionThread.
> The ::CancelIo function is called, freeing the 'serve' thread with an 
> unsuccessful OvelappedResult which then throws and terminate the server by 
> deleting the serverTransport_.
> All this can happen while the call to interrupt has not yet returned, and 
> even the event signaling the execution of the CANCELIO request might not have 
> been caught.
> In our code, this result in unpredictable behavior (mostly detected as 
> stalled).
> It is very hard to reproduce in test environment as it consists in a running 
> threads race condition.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Updated] (THRIFT-4283) TNamedPipeServer race condition in interrupt

2017-08-10 Thread JIRA

 [ 
https://issues.apache.org/jira/browse/THRIFT-4283?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jean-Noël Goor updated THRIFT-4283:
---
Attachment: thrift-4283-TNamedPipeServer-race-condition.patch

> TNamedPipeServer race condition in interrupt
> 
>
> Key: THRIFT-4283
> URL: https://issues.apache.org/jira/browse/THRIFT-4283
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Library
>Affects Versions: 0.10.0
> Environment: Windows
>Reporter: Jean-Noël Goor
> Attachments: thrift-4283-TNamedPipeServer-race-condition.patch
>
>
> In TNamedPipeServer, there exists a race condition between the 'serve' thread 
> and another thread that would attempt to interrupt it.
> While the 'serve' thread is waiting for clients to connect, it is blocked on 
> a GetOverlappedResult call.
> To stop the server, another thread calls the interrupt function that submits 
> a CANCELIO operation to the TOverlappedSubmissionThread.
> The ::CancelIo function is called, freeing the 'serve' thread with an 
> unsuccessful OvelappedResult which then throws and terminate the server by 
> deleting the serverTransport_.
> All this can happen while the call to interrupt has not yet returned, and 
> even the event signaling the execution of the CANCELIO request might not have 
> been caught.
> In our code, this result in unpredictable behavior (mostly detected as 
> stalled).
> It is very hard to reproduce in test environment as it consists in a running 
> threads race condition.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Created] (THRIFT-4283) TNamedPipeServer race condition in interrupt

2017-08-10 Thread JIRA
Jean-Noël Goor created THRIFT-4283:
--

 Summary: TNamedPipeServer race condition in interrupt
 Key: THRIFT-4283
 URL: https://issues.apache.org/jira/browse/THRIFT-4283
 Project: Thrift
  Issue Type: Bug
  Components: C++ - Library
Affects Versions: 0.10.0
 Environment: Windows
Reporter: Jean-Noël Goor


In TNamedPipeServer, there exists a race condition between the 'serve' thread 
and another thread that would attempt to interrupt it.

While the 'serve' thread is waiting for clients to connect, it is blocked on a 
GetOverlappedResult call.

To stop the server, another thread calls the interrupt function that submits a 
CANCELIO operation to the TOverlappedSubmissionThread.

The ::CancelIo function is called, freeing the 'serve' thread with an 
unsuccessful OvelappedResult which then throws and terminate the server by 
deleting the serverTransport_.

All this can happen while the call to interrupt has not yet returned, and even 
the event signaling the execution of the CANCELIO request might not have been 
caught.

In our code, this result in unpredictable behavior (mostly detected as stalled).

It is very hard to reproduce in test environment as it consists in a running 
threads race condition.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)