[jira] [Commented] (THRIFT-3867) Specify BinaryProtocol and CompactProtocol

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-3867:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1036
  
Thanks for the reminder. Actually, I like it. Too bad it's closed now.


> Specify BinaryProtocol and CompactProtocol
> --
>
> Key: THRIFT-3867
> URL: https://issues.apache.org/jira/browse/THRIFT-3867
> Project: Thrift
>  Issue Type: Documentation
>  Components: Documentation
>Reporter: Erik van Oosten
>
> It would be nice when the protocol(s) would be specified somewhere. This 
> should improve communication between developers, but also opens the way for 
> alternative implementations so that Thrift can thrive even better.
> I have a fairly complete description of the BinaryProtocol and 
> CompactProtocol which I will submit as a patch for further review and 
> discussion.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift issue #1036: THRIFT-3867 Specify BinaryProtocol and CompactProtocol

2016-09-20 Thread Jens-G
Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1036
  
Thanks for the reminder. Actually, I like it. Too bad it's closed now.


---
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-3867) Specify BinaryProtocol and CompactProtocol

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-3867:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1036
  
Why?


> Specify BinaryProtocol and CompactProtocol
> --
>
> Key: THRIFT-3867
> URL: https://issues.apache.org/jira/browse/THRIFT-3867
> Project: Thrift
>  Issue Type: Documentation
>  Components: Documentation
>Reporter: Erik van Oosten
>
> It would be nice when the protocol(s) would be specified somewhere. This 
> should improve communication between developers, but also opens the way for 
> alternative implementations so that Thrift can thrive even better.
> I have a fairly complete description of the BinaryProtocol and 
> CompactProtocol which I will submit as a patch for further review and 
> discussion.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift issue #1036: THRIFT-3867 Specify BinaryProtocol and CompactProtocol

2016-09-20 Thread Jens-G
Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1036
  
Why?


---
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] [Closed] (THRIFT-1108) SSL support for the Ruby library

2016-09-20 Thread Jake Farrell (JIRA)

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

Jake Farrell closed THRIFT-1108.

   Resolution: Fixed
Fix Version/s: 0.10.0

Thanks for the update and tests, committed

> SSL support for the Ruby library 
> -
>
> Key: THRIFT-1108
> URL: https://issues.apache.org/jira/browse/THRIFT-1108
> Project: Thrift
>  Issue Type: Improvement
>  Components: Ruby - Library
>Affects Versions: 0.6
>Reporter: Alex
>Assignee: Mansi
>Priority: Minor
>  Labels: ssl
> Fix For: 0.10.0
>
> Attachments: ssl_support.diff
>
>
> Attached are modified versions of the socket and server_socket files which 
> include SSL support. I do not consider these implementations complete, 
> however they work well for my purposes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-1108) SSL support for the Ruby library

2016-09-20 Thread Jake Farrell (JIRA)

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

Jake Farrell updated THRIFT-1108:
-
Assignee: Mansi

> SSL support for the Ruby library 
> -
>
> Key: THRIFT-1108
> URL: https://issues.apache.org/jira/browse/THRIFT-1108
> Project: Thrift
>  Issue Type: Improvement
>  Components: Ruby - Library
>Affects Versions: 0.6
>Reporter: Alex
>Assignee: Mansi
>Priority: Minor
>  Labels: ssl
> Attachments: ssl_support.diff
>
>
> Attached are modified versions of the socket and server_socket files which 
> include SSL support. I do not consider these implementations complete, 
> however they work well for my purposes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-1108) SSL support for the Ruby library

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-1108:


Github user jfarrell closed the pull request at:

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


> SSL support for the Ruby library 
> -
>
> Key: THRIFT-1108
> URL: https://issues.apache.org/jira/browse/THRIFT-1108
> Project: Thrift
>  Issue Type: Improvement
>  Components: Ruby - Library
>Affects Versions: 0.6
>Reporter: Alex
>Priority: Minor
>  Labels: ssl
> Attachments: ssl_support.diff
>
>
> Attached are modified versions of the socket and server_socket files which 
> include SSL support. I do not consider these implementations complete, 
> however they work well for my purposes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-1108) SSL support for the Ruby library

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-1108:


Github user jfarrell commented on the issue:

https://github.com/apache/thrift/pull/1091
  
Thanks for the patch,Committed, 04e6f62


> SSL support for the Ruby library 
> -
>
> Key: THRIFT-1108
> URL: https://issues.apache.org/jira/browse/THRIFT-1108
> Project: Thrift
>  Issue Type: Improvement
>  Components: Ruby - Library
>Affects Versions: 0.6
>Reporter: Alex
>Priority: Minor
>  Labels: ssl
> Attachments: ssl_support.diff
>
>
> Attached are modified versions of the socket and server_socket files which 
> include SSL support. I do not consider these implementations complete, 
> however they work well for my purposes.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift pull request #1091: THRIFT-1108: Adding SSL support for ruby

2016-09-20 Thread jfarrell
Github user jfarrell closed the pull request at:

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


---
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 #1091: THRIFT-1108: Adding SSL support for ruby

2016-09-20 Thread jfarrell
Github user jfarrell commented on the issue:

https://github.com/apache/thrift/pull/1091
  
Thanks for the patch,Committed, 04e6f62


---
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-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Description: 
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code block first, {{ThreadManager}}'s {{removeWorker}} 
method unblocks, and eventually {{ThreadManage}}r's {{join}} returns and the 
object is destructed. When the other thread reaches the synchronized block 
above, it will crash, as the manager is not around anymore.

Besides, {{ThreadManager}} never joins its threads.

Attached is a small fix that addresses these problems.

  was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, {{ThreadManager}} never joins its threads.

Attached is a small fix that addresses these problems.


> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code block first, {{ThreadManager}}'s {{removeWorker}} 
> method unblocks, and eventually {{ThreadManage}}r's {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, {{ThreadManager}} never joins its threads.
> Attached is a small fix that addresses these problems.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift pull request #1092: THRIFT-948: Adding SSL support for php

2016-09-20 Thread jfarrell
Github user jfarrell closed the pull request at:

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


---
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] [Closed] (THRIFT-948) SSL socket support for PHP

2016-09-20 Thread Jake Farrell (JIRA)

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

Jake Farrell closed THRIFT-948.
---
   Resolution: Fixed
Fix Version/s: 0.10.0

Committed, thanks for the patch

> SSL socket support for PHP
> --
>
> Key: THRIFT-948
> URL: https://issues.apache.org/jira/browse/THRIFT-948
> Project: Thrift
>  Issue Type: New Feature
>  Components: PHP - Library
>Reporter: David Reiss
>Assignee: Mansi
>Priority: Minor
> Fix For: 0.10.0
>
> Attachments: php-ssl-0.9.2.patch, php-ssl.patch
>
>
> This had some basic testing, but was never used in production.  If someone 
> wants to implement this feature, they can start with this patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift issue #1092: THRIFT-948: Adding SSL support for php

2016-09-20 Thread jfarrell
Github user jfarrell commented on the issue:

https://github.com/apache/thrift/pull/1092
  
commit 311c984


---
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-948) SSL socket support for PHP

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-948:
---

Github user jfarrell commented on the issue:

https://github.com/apache/thrift/pull/1092
  
commit 311c984


> SSL socket support for PHP
> --
>
> Key: THRIFT-948
> URL: https://issues.apache.org/jira/browse/THRIFT-948
> Project: Thrift
>  Issue Type: New Feature
>  Components: PHP - Library
>Reporter: David Reiss
>Assignee: Mansi
>Priority: Minor
> Fix For: 0.10.0
>
> Attachments: php-ssl-0.9.2.patch, php-ssl.patch
>
>
> This had some basic testing, but was never used in production.  If someone 
> wants to implement this feature, they can start with this patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-948) SSL socket support for PHP

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-948:
---

Github user jfarrell closed the pull request at:

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


> SSL socket support for PHP
> --
>
> Key: THRIFT-948
> URL: https://issues.apache.org/jira/browse/THRIFT-948
> Project: Thrift
>  Issue Type: New Feature
>  Components: PHP - Library
>Reporter: David Reiss
>Assignee: Mansi
>Priority: Minor
> Fix For: 0.10.0
>
> Attachments: php-ssl-0.9.2.patch, php-ssl.patch
>
>
> This had some basic testing, but was never used in production.  If someone 
> wants to implement this feature, they can start with this patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-948) SSL socket support for PHP

2016-09-20 Thread Jake Farrell (JIRA)

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

Jake Farrell updated THRIFT-948:

Assignee: Mansi

> SSL socket support for PHP
> --
>
> Key: THRIFT-948
> URL: https://issues.apache.org/jira/browse/THRIFT-948
> Project: Thrift
>  Issue Type: New Feature
>  Components: PHP - Library
>Reporter: David Reiss
>Assignee: Mansi
>Priority: Minor
> Attachments: php-ssl-0.9.2.patch, php-ssl.patch
>
>
> This had some basic testing, but was never used in production.  If someone 
> wants to implement this feature, they can start with this patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Comment Edited] (THRIFT-3928) CPP generator doesn`t generate implementations of constructors, operators and setters for function helpers

2016-09-20 Thread Vadrot Romain (JIRA)

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

Vadrot Romain edited comment on THRIFT-3928 at 9/20/16 2:56 PM:


The proposed patch is a good start but is not complete as it does not generate 
implementation of assignment operator for result objects.

It seems something went wrong. 
In 0.9.0:
* default constructor was declared and implemented,
* copy constructor and assignment operator were not declared (so assumed 
generated by compiler)
* setters were declared and implemented.

Now in 0.9.3:
* default constructor is declared and implemented,
* copy constructor and assignment operator are declared and not implemented (so 
not generated by compiler), so link error when used
* setters are declared and not implemented, so link error when used


was (Author: romain.vadrot):
The proposed patch is a good start but is not complete as it does not generate 
implementation of assignment operator for result objects.

It seems something went wrong. 
In 0.9.0:
* default constructor was declared and implemented,
* copy constructor and assignment operator were not declared (so generated by 
compiler)
* setters were declared and implemented.

Now in 0.9.3:
* default constructor is declared and implemented,
* copy constructor and assignment operator are declared and not implemented (so 
not generated by compiler), so link error when used
* setters are declared and not implemented, so link error when used

> CPP generator doesn`t generate implementations of constructors, operators and 
> setters for function helpers
> --
>
> Key: THRIFT-3928
> URL: https://issues.apache.org/jira/browse/THRIFT-3928
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Compiler
>Affects Versions: 0.9.3
>Reporter: Denis 
>  Labels: easyfix
> Attachments: thrift_cpp_generator.patch
>
>
> cpp generator  doesn`t  generate implementations of constructors, operators  
> for function and service helpers in *Service.cpp 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-3928) CPP generator doesn`t generate implementations of constructors, operators and setters for function helpers

2016-09-20 Thread Vadrot Romain (JIRA)

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

Vadrot Romain commented on THRIFT-3928:
---

The proposed patch is a good start but is not complete as it does not generate 
implementation of assignment operator for result objects.

It seems something went wrong. 
In 0.9.0:
* default constructor was declared and implemented,
* copy constructor and assignment operator were not declared (so generated by 
compiler)
* setters were declared and implemented.

Now in 0.9.3:
* default constructor is declared and implemented,
* copy constructor and assignment operator are declared and not implemented (so 
not generated by compiler), so link error when used
* setters are declared and not implemented, so link error when used

> CPP generator doesn`t generate implementations of constructors, operators and 
> setters for function helpers
> --
>
> Key: THRIFT-3928
> URL: https://issues.apache.org/jira/browse/THRIFT-3928
> Project: Thrift
>  Issue Type: Bug
>  Components: C++ - Compiler
>Affects Versions: 0.9.3
>Reporter: Denis 
>  Labels: easyfix
> Attachments: thrift_cpp_generator.patch
>
>
> cpp generator  doesn`t  generate implementations of constructors, operators  
> for function and service helpers in *Service.cpp 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-948) SSL socket support for PHP

2016-09-20 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on THRIFT-948:
---

GitHub user mansinahar opened a pull request:

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

THRIFT-948: Adding SSL support for php



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

$ git pull https://github.com/mansinahar/thrift thrift-php-ssl

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

https://github.com/apache/thrift/pull/1092.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 #1092


commit 74bc55f9ebd998ab1d8961d26c36838fba8ed094
Author: Mansi Nahar 
Date:   2016-09-19T13:33:30Z

THRIFT-948: Adding SSL support for php




> SSL socket support for PHP
> --
>
> Key: THRIFT-948
> URL: https://issues.apache.org/jira/browse/THRIFT-948
> Project: Thrift
>  Issue Type: New Feature
>  Components: PHP - Library
>Reporter: David Reiss
>Priority: Minor
> Attachments: php-ssl-0.9.2.patch, php-ssl.patch
>
>
> This had some basic testing, but was never used in production.  If someone 
> wants to implement this feature, they can start with this patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] thrift pull request #1092: THRIFT-948: Adding SSL support for php

2016-09-20 Thread mansinahar
GitHub user mansinahar opened a pull request:

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

THRIFT-948: Adding SSL support for php



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

$ git pull https://github.com/mansinahar/thrift thrift-php-ssl

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

https://github.com/apache/thrift/pull/1092.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 #1092


commit 74bc55f9ebd998ab1d8961d26c36838fba8ed094
Author: Mansi Nahar 
Date:   2016-09-19T13:33:30Z

THRIFT-948: Adding SSL support for php




---
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 #1036: THRIFT-3867 Specify BinaryProtocol and CompactProtocol

2016-09-20 Thread erikvanoosten
Github user erikvanoosten commented on the issue:

https://github.com/apache/thrift/pull/1036
  
Closing this due to lack of interest of repo owners.


---
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 #1036: THRIFT-3867 Specify BinaryProtocol and CompactPro...

2016-09-20 Thread erikvanoosten
Github user erikvanoosten closed the pull request at:

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


---
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-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Description: 
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, {{ThreadManager}} never joins its threads.

Attached is a small fix that solves these problems.

  was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that solves these problems.


> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code first, and the ThreadManager's {{removeWorker}} 
> method unblocks and eventually the ThreadManager's {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, {{ThreadManager}} never joins its threads.
> Attached is a small fix that solves these problems.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Description: 
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, {{ThreadManager}} never joins its threads.

Attached is a small fix that addresses these problems.

  was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, {{ThreadManager}} never joins its threads.

Attached is a small fix that solves these problems.


> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code first, and the ThreadManager's {{removeWorker}} 
> method unblocks and eventually the ThreadManager's {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, {{ThreadManager}} never joins its threads.
> Attached is a small fix that addresses these problems.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Description: 
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that solves these problems.

  was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that alleviates these problems.


> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code first, and the ThreadManager's {{removeWorker}} 
> method unblocks and eventually the ThreadManager's {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, the ThreadManager never joins its threads.
> Attached is a small fix that solves these problems.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Description: 
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
thread that detects {{workerCount_ == workerMaxCount_}} notifies 
{{removeWorker}}. The {{run}} method has the following additional code that is 
executed at the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object is 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that alleviates these problems.

  was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. In the run method, the last thread that detects 
{{workerCount_ == workerMaxCount_}} notifies the {{removeWorker}} method. 
However, the run method has the following additional code that is executed at 
the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that alleviates the problem.


> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code first, and the ThreadManager's {{removeWorker}} 
> method unblocks and eventually the ThreadManager's {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, the ThreadManager never joins its threads.
> Attached is a small fix that alleviates these problems.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (THRIFT-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA
Buğra Gedik created THRIFT-3932:
---

 Summary: C++ ThreadManager has a rare termination race
 Key: THRIFT-3932
 URL: https://issues.apache.org/jira/browse/THRIFT-3932
 Project: Thrift
  Issue Type: Bug
Reporter: Buğra Gedik
 Attachments: thrift-patch

{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
!= workerMaxCount_)}}. In the run method, the last thread that detects 
{{workerCount_ == workerMaxCount_}} notifies the {{removeWorker}} method. 
However, the run method has the following additional code that is executed at 
the very end:

{code}
{
  Synchronized s(manager_->workerMonitor_);
  manager_->deadWorkers_.insert(this->thread());
  if (notifyManager) {
manager_->workerMonitor_.notify();
  }
}
{code}

This is an independent synchronized block. Now assume 2 threads. One of them 
has {{notifyManager=true}} as it detected the {{workerCount_ == 
workerMaxCount_}} condition earlier. It is possible that this thread gets to 
execute  the above code first, and the ThreadManager's {{removeWorker}} method 
unblocks and eventually the ThreadManager's {{join}} returns and the object 
destructed. When the other thread reaches the synchronized block above, it will 
crash, as the manager is not around anymore.

Besides, the ThreadManager never joins its threads.

Attached is a small fix that alleviates the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-3925) Client service recieves string instead of buffer when using TCompactProtocol with node.js library.

2016-09-20 Thread Roman Poliakov (JIRA)

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

Roman Poliakov updated THRIFT-3925:
---
Description: 
Node.js implementation of TCompactProtocol instead of returning raw Buffer, 
converts it to string, which leads to data corruption. 

At the line: 
https://github.com/apache/thrift/blob/0.9.3/lib/nodejs/lib/thrift/compact_protocol.js#L764

you can see that it uses "readString" method of the transport instead of using 
"read".


  was:
Node.js implementation of TCompactProtocol instead of returning raw Buffer 
converts it to string which leads to data corruption. 

At the line: 
https://github.com/apache/thrift/blob/0.9.3/lib/nodejs/lib/thrift/compact_protocol.js#L764

you can see that it uses "readString" method of the transport instead of using 
"read".



> Client service recieves string instead of buffer when using TCompactProtocol 
> with node.js library.
> --
>
> Key: THRIFT-3925
> URL: https://issues.apache.org/jira/browse/THRIFT-3925
> Project: Thrift
>  Issue Type: Bug
>  Components: Node.js - Library
>Affects Versions: 0.9.3
>Reporter: Roman Poliakov
>Priority: Critical
> Attachments: 
> Fixed_issue_with_TCompactProtocol_returning_string_instead_of_Buffer_for_service_function_.patch
>
>
> Node.js implementation of TCompactProtocol instead of returning raw Buffer, 
> converts it to string, which leads to data corruption. 
> At the line: 
> https://github.com/apache/thrift/blob/0.9.3/lib/nodejs/lib/thrift/compact_protocol.js#L764
> you can see that it uses "readString" method of the transport instead of 
> using "read".



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race

2016-09-20 Thread JIRA

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

Buğra Gedik updated THRIFT-3932:

Attachment: thrift-patch

> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
>  Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. In the run method, the last thread that detects 
> {{workerCount_ == workerMaxCount_}} notifies the {{removeWorker}} method. 
> However, the run method has the following additional code that is executed at 
> the very end:
> {code}
> {
>   Synchronized s(manager_->workerMonitor_);
>   manager_->deadWorkers_.insert(this->thread());
>   if (notifyManager) {
> manager_->workerMonitor_.notify();
>   }
> }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code first, and the ThreadManager's {{removeWorker}} 
> method unblocks and eventually the ThreadManager's {{join}} returns and the 
> object destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, the ThreadManager never joins its threads.
> Attached is a small fix that alleviates the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (THRIFT-3925) Client service recieves string instead of buffer when using TCompactProtocol with node.js library.

2016-09-20 Thread Roman Poliakov (JIRA)

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

Roman Poliakov commented on THRIFT-3925:


Can this patch be applied? The version of library in npm repository has this 
bug.

> Client service recieves string instead of buffer when using TCompactProtocol 
> with node.js library.
> --
>
> Key: THRIFT-3925
> URL: https://issues.apache.org/jira/browse/THRIFT-3925
> Project: Thrift
>  Issue Type: Bug
>  Components: Node.js - Library
>Affects Versions: 0.9.3
>Reporter: Roman Poliakov
>Priority: Critical
> Attachments: 
> Fixed_issue_with_TCompactProtocol_returning_string_instead_of_Buffer_for_service_function_.patch
>
>
> Node.js implementation of TCompactProtocol instead of returning raw Buffer 
> converts it to string which leads to data corruption. 
> At the line: 
> https://github.com/apache/thrift/blob/0.9.3/lib/nodejs/lib/thrift/compact_protocol.js#L764
> you can see that it uses "readString" method of the transport instead of 
> using "read".



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)