Github user tpcwang commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-215848959
LGTM. Thanks for fixing this issue.
---
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
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61612686
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61601611
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61589086
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61581424
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61558381
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r61534321
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-215502049
Looking for an advocate who would consider merging this fix despite the
busted Apache Jenkins environment result.
---
If your project is set up for it, you can reply
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-214391825
Looking for an advocate to consider merging this. It passed all three
builds with the exception of something that happened on the apache jenkins
where a cppcheck file
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60614631
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60614134
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60612338
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60591056
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60590542
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r60227644
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,69 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59996235
--- Diff: lib/cpp/test/TServerIntegrationTest.cpp ---
@@ -152,7 +155,10 @@ class TServerIntegrationTestFixture {
new
Github user tpcwang commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-211137935
I think this change looks good for the most part, maybe we can get one of
the maintainers to look at it?
---
If your project is set up for it, you can reply to this
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59996087
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59995963
--- Diff: lib/cpp/src/thrift/concurrency/ThreadManager.cpp ---
@@ -421,7 +416,7 @@ void ThreadManager::Impl::removeWorker(size_t value) {
{
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-209039437
@jfarrell Could you look into why the Jenkins build for apache thinks that
a c_glib file needs to be merged manually and/or has an open pull request on
the apache build
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59410629
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.h ---
@@ -83,12 +88,56 @@ class TThreadedServer : public TServerFramework {
virtual void serve();
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59390673
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.h ---
@@ -83,12 +88,56 @@ class TThreadedServer : public TServerFramework {
virtual void serve();
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59390310
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59306037
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.h ---
@@ -83,12 +88,56 @@ class TThreadedServer : public TServerFramework {
virtual void serve();
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59306085
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59305327
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59304879
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.h ---
@@ -83,12 +88,56 @@ class TThreadedServer : public TServerFramework {
virtual void serve();
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r59304884
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -90,31 +95,71 @@ TThreadedServer::~TThreadedServer() {
}
void
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-208513794
I'm starting to believe that the TThreadServerPool implementation may have
the same issue. The cores I have been able to produce through repeated testing
are showing a
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-208013575
I ran the test that failed in the apache build in a loop and finally ended
up in a deadlock situation, so I am looking at that.
---
If your project is set up for it,
Github user jeking3 commented on the pull request:
https://github.com/apache/thrift/pull/980#issuecomment-207675424
I did not touch "go" or "py" implementations and that was the reason for
the failure in #2467.2. #2467.1 failed in three tests including go-cpp/json
however given
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r58851243
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -92,29 +97,42 @@ TThreadedServer::~TThreadedServer() {
void TThreadedServer::serve() {
Github user tpcwang commented on a diff in the pull request:
https://github.com/apache/thrift/pull/980#discussion_r58814042
--- Diff: lib/cpp/src/thrift/server/TThreadedServer.cpp ---
@@ -92,29 +97,42 @@ TThreadedServer::~TThreadedServer() {
void TThreadedServer::serve() {
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/980
THRIFT-3768: ensure TThreadedServer guarantees the lifetime of the client
TThreadedServer now guarantees the lifetime of the TConnectedClient.
You can merge this pull request into a Git repository
34 matches
Mail list logo
