[jira] [Comment Edited] (THRIFT-3507) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15525258#comment-15525258 ] Martin Wilck edited comment on THRIFT-3507 at 9/27/16 6:43 AM: --- Hello Jens, AFAICS you committed the changes only to the master branch. This issue (3507) was made against 0.9.3. Are there any plans to add the proxy support to 0.9.3? Or will 3rd party projects need to update to 1.0.x to benefit from this feature? Martin was (Author: mwilck): Hello Jens, AFAICS you committed the changes only to the master branch. This issue (0.9.3) was made against 0.9.3. Are there any plans to add the proxy support to 0.9.3? Or will 3rd party projects need to update to 1.0.x to benefit from this feature? Martin > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3507 > URL: https://issues.apache.org/jira/browse/THRIFT-3507 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 >Reporter: Fabricio Oliveira >Assignee: Martin Wilck >Priority: Critical > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > THRIFT-3507-1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3507) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15525258#comment-15525258 ] Martin Wilck commented on THRIFT-3507: -- Hello Jens, AFAICS you committed the changes only to the master branch. This issue (0.9.3) was made against 0.9.3. Are there any plans to add the proxy support to 0.9.3? Or will 3rd party projects need to update to 1.0.x to benefit from this feature? Martin > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3507 > URL: https://issues.apache.org/jira/browse/THRIFT-3507 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 >Reporter: Fabricio Oliveira >Assignee: Martin Wilck >Priority: Critical > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > THRIFT-3507-1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] thrift issue #1096: fix snprintf error in TServerSocket.cpp under vs2013
Github user CompileMaster commented on the issue: https://github.com/apache/thrift/pull/1096 @Jens-G windows+msvc has no "snprintf" funtion, but has "_snprintf" funtion. so should use "_snprintf" function replace the "snprintf" function. --- 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 #1096: fix snprintf error in TServerSocket.cpp under vs2013
Github user CompileMaster commented on the issue: https://github.com/apache/thrift/pull/1096 Compiling under VS2013 error C3861: âsnprintfâ: Can't find the identifier , line 282, 527, 528 in TServerSocket.cpp --- 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-3855) In the go simple server, if Stop() is called multiple times it hangs
[
https://issues.apache.org/jira/browse/THRIFT-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15525072#comment-15525072
]
Paul Finkelshteyn commented on THRIFT-3855:
---
Thank you! Now we're waiting for 0.10 version!
> In the go simple server, if Stop() is called multiple times it hangs
>
>
> Key: THRIFT-3855
> URL: https://issues.apache.org/jira/browse/THRIFT-3855
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
>Affects Versions: 0.9.3
>Reporter: James E. King, III
>Assignee: Paul Finkelshteyn
>Priority: Minor
> Fix For: 0.10.0
>
>
> From the submitter huaiwan:
> {quote}
> huaiyun commented 18 hours ago
> When Stop() is called twice or more, and no new connection accepted from
> AcceptLoop(), the Stop() will be blocked because the quit channel is full.
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3855) In the go simple server, if Stop() is called multiple times it hangs
[
https://issues.apache.org/jira/browse/THRIFT-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-3855:
---
Assignee: Paul Finkelshteyn (was: James E. King, III)
> In the go simple server, if Stop() is called multiple times it hangs
>
>
> Key: THRIFT-3855
> URL: https://issues.apache.org/jira/browse/THRIFT-3855
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
>Affects Versions: 0.9.3
>Reporter: James E. King, III
>Assignee: Paul Finkelshteyn
>Priority: Minor
> Fix For: 0.10.0
>
>
> From the submitter huaiwan:
> {quote}
> huaiyun commented 18 hours ago
> When Stop() is called twice or more, and no new connection accepted from
> AcceptLoop(), the Stop() will be blocked because the quit channel is full.
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-3855) In the go simple server, if Stop() is called multiple times it hangs
[
https://issues.apache.org/jira/browse/THRIFT-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15524043#comment-15524043
]
ASF GitHub Bot commented on THRIFT-3855:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1094
> In the go simple server, if Stop() is called multiple times it hangs
>
>
> Key: THRIFT-3855
> URL: https://issues.apache.org/jira/browse/THRIFT-3855
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
>Affects Versions: 0.9.3
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Minor
> Fix For: 0.10.0
>
>
> From the submitter huaiwan:
> {quote}
> huaiyun commented 18 hours ago
> When Stop() is called twice or more, and no new connection accepted from
> AcceptLoop(), the Stop() will be blocked because the quit channel is full.
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Resolved] (THRIFT-3855) In the go simple server, if Stop() is called multiple times it hangs
[
https://issues.apache.org/jira/browse/THRIFT-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-3855.
Resolution: Fixed
Committed
> In the go simple server, if Stop() is called multiple times it hangs
>
>
> Key: THRIFT-3855
> URL: https://issues.apache.org/jira/browse/THRIFT-3855
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
>Affects Versions: 0.9.3
>Reporter: James E. King, III
>Assignee: James E. King, III
>Priority: Minor
> Fix For: 0.10.0
>
>
> From the submitter huaiwan:
> {quote}
> huaiyun commented 18 hours ago
> When Stop() is called twice or more, and no new connection accepted from
> AcceptLoop(), the Stop() will be blocked because the quit channel is full.
> {quote}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request #1094: Fixes THRIFT-3855
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1094 --- 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] [Resolved] (THRIFT-3507) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3507?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer resolved THRIFT-3507. Resolution: Duplicate Assignee: Martin Wilck Fix Version/s: 0.10.0 Closed due to being a technical duplicate to THRIFT-3798, see [this comment|https://issues.apache.org/jira/browse/THRIFT-3507?focusedCommentId=15522369&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15522369] > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3507 > URL: https://issues.apache.org/jira/browse/THRIFT-3507 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 >Reporter: Fabricio Oliveira >Assignee: Martin Wilck >Priority: Critical > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > THRIFT-3507-1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (THRIFT-3798) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3798?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-3798: --- Priority: Major (was: Critical) > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3798 > URL: https://issues.apache.org/jira/browse/THRIFT-3798 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3, 1.0 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 > Linux + Python 2.7.x > Any OS + Python 2.7.x > Any OS + Python 3.x >Reporter: Martin Wilck >Assignee: Martin Wilck > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > 0002-Python-THttpClient-Support-proxy-authorization.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (THRIFT-3798) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3798?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-3798: --- Summary: THttpClient does not use proxy from http_proxy, https_proxy environment variables (was: CLONE - THttpClient does not use proxy from http_proxy, https_proxy environment variables) > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3798 > URL: https://issues.apache.org/jira/browse/THRIFT-3798 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3, 1.0 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 > Linux + Python 2.7.x > Any OS + Python 2.7.x > Any OS + Python 3.x >Reporter: Martin Wilck >Assignee: Martin Wilck >Priority: Critical > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > 0002-Python-THttpClient-Support-proxy-authorization.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Resolved] (THRIFT-3798) CLONE - THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3798?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer resolved THRIFT-3798. Resolution: Fixed Assignee: Martin Wilck Fix Version/s: 0.10.0 Committed. > CLONE - THttpClient does not use proxy from http_proxy, https_proxy > environment variables > - > > Key: THRIFT-3798 > URL: https://issues.apache.org/jira/browse/THRIFT-3798 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3, 1.0 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 > Linux + Python 2.7.x > Any OS + Python 2.7.x > Any OS + Python 3.x >Reporter: Martin Wilck >Assignee: Martin Wilck >Priority: Critical > Fix For: 0.10.0 > > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > 0002-Python-THttpClient-Support-proxy-authorization.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] thrift issue #1096: fix snprintf error in TServerSocket.cpp under vs2013
Github user Jens-G commented on the issue: https://github.com/apache/thrift/pull/1096 Hi @CompileMaster, a bit more information about the problem would be nice. Aside from that, we have a nice [contribution guide](http://thrift.apache.org/docs/HowToContribute) that we would like you to follow. --- 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] [Comment Edited] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523824#comment-15523824 ] James E. King, III edited comment on THRIFT-1310 at 9/26/16 6:33 PM: - It sounds like the issue is in the server side. If a client can disconnect, reconnect, and get a response from a previous connection that is a major security issue, because then any other client that connects (not necessarily the one that "re"connects) could get data that doesn't belong to it. This should not be cheched on the client with seqId matching... this is a server-side defect and a high priority, security issue. Disconnecting should ensure that any outstanding requests at the time of disconnect are either canceled or run to completion however their response should never be accessible because the client that started them has disconnected. Most servers only support one concurrent request per client. I haven't looked at the PHP library yet so I don't know, but for servers that support multiple requests from a client connection to be running simultaneously, the seqId is used to allow the client to match a response to a reply. Still, that should only be valid on a single connection. was (Author: jking3): It sounds like the issue is in the server side. If a client can disconnect, reconnect, and get a response from a previous connection that is a major security issue, because then any other client that connects (not necessarily the one that "re"connects) could get data that doesn't belong to it. This should not be cheched on the client with seqId matching... this is a server-side defect and a high priority, security issue. Disconnecting should ensure that any outstanding requests at the time of disconnect are either canceled or run to completion however their response should never be accessible because the client that started them has disconnected. > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian >Priority: Critical > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Comment Edited] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523824#comment-15523824 ] James E. King, III edited comment on THRIFT-1310 at 9/26/16 6:29 PM: - It sounds like the issue is in the server side. If a client can disconnect, reconnect, and get a response from a previous connection that is a major security issue, because then any other client that connects (not necessarily the one that "re"connects) could get data that doesn't belong to it. This should not be cheched on the client with seqId matching... this is a server-side defect and a high priority, security issue. Disconnecting should ensure that any outstanding requests at the time of disconnect are either canceled or run to completion however their response should never be accessible because the client that started them has disconnected. was (Author: jking3): It sounds like the issue is in the server side. If a client can disconnect, reconnect, and get a response from a previous connection that is a major security issue, because then any other client that connects (not necessarily the one that "re"connects) could get data that doesn't belong to it. This should not be cheched on the client with seqId matching... this is a server-side defect and a high priority, security issue. > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian >Priority: Critical > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James E. King, III updated THRIFT-1310: --- Priority: Critical (was: Major) > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian >Priority: Critical > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James E. King, III updated THRIFT-1310: --- Labels: security-issue (was: ) > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James E. King, III updated THRIFT-1310: --- Component/s: (was: PHP - Compiler) PHP - Library > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-1310) Generate PHP client code not check sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523824#comment-15523824 ] James E. King, III commented on THRIFT-1310: It sounds like the issue is in the server side. If a client can disconnect, reconnect, and get a response from a previous connection that is a major security issue, because then any other client that connects (not necessarily the one that "re"connects) could get data that doesn't belong to it. This should not be cheched on the client with seqId matching... this is a server-side defect and a high priority, security issue. > Generate PHP client code not check sequence ID in messages > -- > > Key: THRIFT-1310 > URL: https://issues.apache.org/jira/browse/THRIFT-1310 > Project: Thrift > Issue Type: Bug > Components: PHP - Library >Affects Versions: 0.7 >Reporter: Fang Jian > Labels: security-issue > Attachments: t_php_generator.patch > > > The PHP client code not check sequence ID in messages, when client connect > timeout, the return of results are out of sequence. I try to fix this by > throwing a exception when sequence ID not equal. Patch file is listed below. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523411#comment-15523411
]
Buğra Gedik commented on THRIFT-3932:
-
Btw, the change I made to the initialization of {{workerMonitor_}} is most
likely not needed, but does not hurt. I'll appreciate if you review and update
as you see fit.
> 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523383#comment-15523383
]
Buğra Gedik commented on THRIFT-3932:
-
Please go ahead.
> 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523358#comment-15523358
]
James E. King, III commented on THRIFT-3932:
I am planning on reviewing this, apply it locally, and if successful submitting
a pull request for it unless you beat me to it.
> 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
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 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request #1093: Add persistence patch
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80496721
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+}
--- End diff --
I would suggest that the issue lies in the server implementation in PHP
based on your description. If a client can disconnect, then a new client
connects and it receives the response from the server originally intended for
the client that disconnected then the server is misbehaving and a massive
security hole. Perhaps it keeps a list of responses based on IP address of
something (I haven't looked). In any case, if you look at the C++ server, each
connection is handled by a single thread, so there can only be one outstanding
request at a time. If the client disconnects, the thread completes the request
and then fails to send the reply, and ends. There is no possibility that
another client would receive the reply. I would recommend instead of trying to
solve the issue on the client side that fixing the root cause on the server
side would be better as it would be much more secure. I don't like the notion
that someone can connect to a thrift PHP server and possibly receive con
fidential information originally destined for another connection.
---
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
[
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 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: 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 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
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 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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: patch.rtf
> 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: patch.rtf)
> 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: patch.rtf
> 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
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 {{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 patch.
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 block first, {{ThreadManager}}'s {{removeWorker}}
method unblocks, and eventually {{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.
> 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: patch.rtf)
> 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
>
> {{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 {{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 patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request #1096: Update TServerSocket.cpp
GitHub user CompileMaster opened a pull request: https://github.com/apache/thrift/pull/1096 Update TServerSocket.cpp fix snprintf error in vs2013 You can merge this pull request into a Git repository by running: $ git pull https://github.com/CompileMaster/thrift master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1096.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 #1096 commit 54631a202996a7327dfb66cf3f47e8fe62672e5b Author: CompileMaster <12126...@qq.com> Date: 2016-09-26T14:31:26Z Update TServerSocket.cpp fix snprintf error in vs2013 --- 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
[
https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: 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
>
> {{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 {{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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare termination race
[
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 {{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.
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 block first, {{ThreadManager}}'s {{removeWorker}}
method unblocks, and eventually {{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
>
> {{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 {{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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request #1093: Add persistence patch
Github user sstach commented on a diff in the pull request: https://github.com/apache/thrift/pull/1093#discussion_r80483149 --- Diff: lib/php/lib/Thrift/Transport/TSocket.php --- @@ -255,6 +255,16 @@ public function close() } /** + * Close the socket and try to + */ + public function reconnect() --- End diff -- Here we need a functionality to close a socket even if it`s persistent and open it again. This way we can react to the situation mentioned in the above comment. --- 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-1310) Generate PHP client code not check sequence ID in messages
[
https://issues.apache.org/jira/browse/THRIFT-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523170#comment-15523170
]
ASF GitHub Bot commented on THRIFT-1310:
Github user sstach commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80482717
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+}
--- End diff --
Okay the seqid_ itself is not the problem. I agree to your statement that
it depends on the implementations of the client or the server.
But here is our problem, we introduced Thrift as a performance boost by
using persistent connections. Now that we use it in production we identified
that the PHP implementation is not robust enough for us. It occurs that the
client Socket gets closed by timeout and the client got an Exception. If
another client requested something he got the response that was meant for the
client which got the timeout.
With the seqid_ we have a robust functionality to check that our java
server returns the correct answer to our question, irrelevant whether its a
rand() or a simple increment.
And if we have a miss match we can react on that and our client can handle
the error by opening a new Socket and request again.
We spend a lot of time to get to the bottom of this problem and the seqid_
seems to be the missing piece we are looking for. We were surprised to find out
that it seems that no one else is running into the same problem. Only a 5 year
old [abandoned issue](https://issues.apache.org/jira/browse/THRIFT-1310) seems
to reference this topic.
Do we miss something about the persistence? Is there another way to
determine if the response matches the request?
> Generate PHP client code not check sequence ID in messages
> --
>
> Key: THRIFT-1310
> URL: https://issues.apache.org/jira/browse/THRIFT-1310
> Project: Thrift
> Issue Type: Bug
> Components: PHP - Compiler
>Affects Versions: 0.7
>Reporter: Fang Jian
> Attachments: t_php_generator.patch
>
>
> The PHP client code not check sequence ID in messages, when client connect
> timeout, the return of results are out of sequence. I try to fix this by
> throwing a exception when sequence ID not equal. Patch file is listed below.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] thrift pull request #1093: Add persistence patch
Github user sstach commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80482717
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+}
--- End diff --
Okay the seqid_ itself is not the problem. I agree to your statement that
it depends on the implementations of the client or the server.
But here is our problem, we introduced Thrift as a performance boost by
using persistent connections. Now that we use it in production we identified
that the PHP implementation is not robust enough for us. It occurs that the
client Socket gets closed by timeout and the client got an Exception. If
another client requested something he got the response that was meant for the
client which got the timeout.
With the seqid_ we have a robust functionality to check that our java
server returns the correct answer to our question, irrelevant whether its a
rand() or a simple increment.
And if we have a miss match we can react on that and our client can handle
the error by opening a new Socket and request again.
We spend a lot of time to get to the bottom of this problem and the seqid_
seems to be the missing piece we are looking for. We were surprised to find out
that it seems that no one else is running into the same problem. Only a 5 year
old [abandoned issue](https://issues.apache.org/jira/browse/THRIFT-1310) seems
to reference this topic.
Do we miss something about the persistence? Is there another way to
determine if the response matches the request?
---
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 #1093: Add persistence patch
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80467634
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+}
--- End diff --
The sequence ID was originally put in place for future expansion of the
servers to allow for multiple outstanding requests from a client. Most servers
do not implement this. For example on the C++ threaded servers there is one
request per client at any given time. The C# server is the same way. The
servers could reply with a seqId that matches the request the client sent,
allowing a client to submit multiple requests, if the servers were written
differently. Most client implementations ignore seqId because they only issue
one request at a time and not all servers fill in the field properly. The
seqId should be handled on the server as a connection-specific unique request
id.
My recommendation would be to follow what other clients do and atomically
increment a number starting at 1. This will be safer than random. It probably
isn't safe to check that the server's response seqId equals the last request
sent. Some servers may not fill in the field. This also limits your client to
one concurrent request and hampers future expansion.
---
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 #1093: Add persistence patch
Github user jeking3 commented on a diff in the pull request: https://github.com/apache/thrift/pull/1093#discussion_r80468446 --- Diff: lib/php/lib/Thrift/Transport/TSocket.php --- @@ -255,6 +255,16 @@ public function close() } /** + * Close the socket and try to + */ + public function reconnect() --- End diff -- I don't see what this has to do with seqId changes and it is incorrect. You would want to call the TSocket::close() and not fclose here. --- 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 #1093: Add persistence patch
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80467137
--- Diff: compiler/cpp/src/generate/t_php_generator.cc ---
@@ -1720,6 +1720,7 @@ void
t_php_generator::generate_service_client(t_service* tservice) {
<< "TBinaryProtocolAccelerated) &&
function_exists('thrift_protocol_write_binary');"
<< endl;
+f_service_client << indent() << "$this->seqid_ = rand();" << endl;
f_service_client << indent() << "if ($bin_accel)" << endl;
--- End diff --
I thign seqId is sequence ID and not sequential ID, however I agree it
should be sequential. Random is less efficient and less effective.
---
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 #1014: Thrift 3839
Github user sstach commented on the issue: https://github.com/apache/thrift/pull/1014 Oh, have overseen that, 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 issue #1014: Thrift 3839
Github user nsuke commented on the issue: https://github.com/apache/thrift/pull/1014 @sstach it's closed because it's merged to the master dd9885e --- 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 #1093: Add persistence patch
Github user sstach commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80425129
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+}
--- End diff --
Thanks for your comment but I'am very interested in what you believe would
be appropriate in this case and how you would solve it, because I think this is
a *major issue* in the php thrift implementation.
Here is mho:
I understand why you complain about the `rand()`.
But a random number is much more useful than an sequential number that
start with 0 and only increment itself. Before `seqid_` wasn't respected at all
and it's used to identify whether the response belongs to my request or not,
and not to determine the order of requests and responses. And lastly, the php
client initiates the `seqid_` with 0 and so it could happen that two clients
communicate with the server at the same time and client A receives the response
of client B and doesn't track that issue because accidentally the `seqid_` is
that one that client A expect.
---
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] [Comment Edited] (THRIFT-3507) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15522369#comment-15522369 ] Martin Wilck edited comment on THRIFT-3507 at 9/26/16 7:51 AM: --- 1. 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch 2. 0002-Python-THttpClient-Support-proxy-authorization.patch I just re-checked the procedure in the above file. The patches apply on current master branch. $ git clone https://github.com/apache/thrift.git $ git branch proxy origin/master $ git checkout proxy $ curl https://issues.apache.org/jira/secure/attachment/12801233/0001-python-THttpClient-Add-support-for-system-proxy-sett.patch | git am $ curl https://issues.apache.org/jira/secure/attachment/12801234/0002-Python-THttpClient-Support-proxy-authorization.patch | git am $ git status On branch proxy Your branch is ahead of 'origin/master' by 2 commits. $ git log --oneline origin/master.. d2bfd42 Python/THttpClient: Support proxy authorization 7ff06a1 python/THttpClient: Add support for system proxy settings was (Author: mwilck): 1. 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch 2. 0002-Python-THttpClient-Support-proxy-authorization.patch I just re-checked the procedure in the above file. The patches apply on current master branch. $ git clone https://github.com/apache/thrift.git $ git branch proxy origin/master $ git checkout proxy $ curl https://issues.apache.org/jira/secure/attachment/12801233/0001-python-THttpClient-Add-support-for-system-proxy-sett.patch | git am $ curl https://issues.apache.org/jira/secure/attachment/12801234/0002-Python-THttpClient-Support-proxy-authorization.patch | git am $ git status On branch proxy1 Your branch is ahead of 'origin/master' by 2 commits. $ git log --oneline origin/master.. d2bfd42 Python/THttpClient: Support proxy authorization 7ff06a1 python/THttpClient: Add support for system proxy settings > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3507 > URL: https://issues.apache.org/jira/browse/THRIFT-3507 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 >Reporter: Fabricio Oliveira >Priority: Critical > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > THRIFT-3507-1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (THRIFT-3507) THttpClient does not use proxy from http_proxy, https_proxy environment variables
[ https://issues.apache.org/jira/browse/THRIFT-3507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15522369#comment-15522369 ] Martin Wilck commented on THRIFT-3507: -- 1. 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch 2. 0002-Python-THttpClient-Support-proxy-authorization.patch I just re-checked the procedure in the above file. The patches apply on current master branch. $ git clone https://github.com/apache/thrift.git $ git branch proxy origin/master $ git checkout proxy $ curl https://issues.apache.org/jira/secure/attachment/12801233/0001-python-THttpClient-Add-support-for-system-proxy-sett.patch | git am $ curl https://issues.apache.org/jira/secure/attachment/12801234/0002-Python-THttpClient-Support-proxy-authorization.patch | git am $ git status On branch proxy1 Your branch is ahead of 'origin/master' by 2 commits. $ git log --oneline origin/master.. d2bfd42 Python/THttpClient: Support proxy authorization 7ff06a1 python/THttpClient: Add support for system proxy settings > THttpClient does not use proxy from http_proxy, https_proxy environment > variables > - > > Key: THRIFT-3507 > URL: https://issues.apache.org/jira/browse/THRIFT-3507 > Project: Thrift > Issue Type: Bug > Components: Python - Library >Affects Versions: 0.9.3 > Environment: Windows 7 + Cygwin x64 + Python 2.7.10 >Reporter: Fabricio Oliveira >Priority: Critical > Attachments: > 0001-python-THttpClient-Add-support-for-system-proxy-sett.patch, > THRIFT-3507-1.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] thrift issue #1014: Thrift 3839
Github user sstach commented on the issue: https://github.com/apache/thrift/pull/1014 @asfgit Why did you closed the PR? --- 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. ---
