diff options
| -rw-r--r-- | ACE/ChangeLog | 33 | ||||
| -rw-r--r-- | ACE/ace/Connector.cpp | 9 | ||||
| -rw-r--r-- | ACE/ace/Svc_Handler.cpp | 6 | ||||
| -rw-r--r-- | ACE/tests/Bug_2609_Regression_Test.cpp | 2 | ||||
| -rw-r--r-- | ACE/tests/Bug_2610_Regression_Test.cpp | 8 | ||||
| -rw-r--r-- | ACE/tests/NonBlocking_Conn_Test.cpp | 30 | ||||
| -rw-r--r-- | ACE/tests/NonBlocking_Conn_Test.h | 1 | ||||
| -rw-r--r-- | ACE/tests/Process_Strategy_Test.cpp | 14 |
8 files changed, 78 insertions, 25 deletions
diff --git a/ACE/ChangeLog b/ACE/ChangeLog index a199b541864..0e42f18a896 100644 --- a/ACE/ChangeLog +++ b/ACE/ChangeLog @@ -1,18 +1,41 @@ +Thu Feb 11 12:18:24 UTC 2010 Vladimir Zykov <vladimir.zykov@prismtech.com> + + * ace/Connector.cpp: + Added a call to remove_reference() for a svc handler owned by + non-blocking connection handler during connector's close(). + + * ace/Svc_Handler.cpp: + Removed the code that removes a reference to itself. Svc_Handler + doesn't own that reference and thus shouldn't remove it. + + * tests/Bug_2609_Regression_Test.cpp: + * tests/NonBlocking_Conn_Test.h: + * tests/Bug_2610_Regression_Test.cpp: + * tests/NonBlocking_Conn_Test.cpp: + Fixed the tests that implicitly assumed ownership of a reference + to a svc handler and didn't free it at the end of the test. + + * tests/Process_Strategy_Test.cpp: + Fixed the test that was broken by my change on + 'Mon Feb 8 16:21:06 UTC 2010'. The test incorrectly assumed + that close_handle() will not be called for svc handlers in + a parent process. + Wed Feb 10 18:39:30 UTC 2010 Martin Corino <mcorino@remedy.nl> * ace/config-g++-common.h: - + Added logic to detect (usable) support for GCC builtin __sync_XXX atomic op functions. - + * ace/Atomic_Op.h: * ace/Atomic_Op.inl: - + Changed autodetection for GCC builtin atomic ops so it will only be used when really wanted. Removed include for stdatomic.h because that has nothing to do with - the __sync_XXX builtins but rather with a *proposed* atomic op - implementation for the C++1x standard. The __sync_XXX builtins are + the __sync_XXX builtins but rather with a *proposed* atomic op + implementation for the C++1x standard. The __sync_XXX builtins are truly intrinsic, i.e. no header files involved. Wed Feb 10 15:38:30 UTC 2010 Johnny Willemsen <jwillemsen@remedy.nl> diff --git a/ACE/ace/Connector.cpp b/ACE/ace/Connector.cpp index 042a7f0415b..0934ac62750 100644 --- a/ACE/ace/Connector.cpp +++ b/ACE/ace/Connector.cpp @@ -736,6 +736,15 @@ ACE_Connector<SVC_HANDLER, ACE_PEER_CONNECTOR_2>::close (void) } SVC_HANDLER *svc_handler = nbch->svc_handler (); + // Since nbch holds a reference to svc_handler we have to + // free it here as there will be no other chance to do it. + ACE_Event_Handler_var ref_count_guard; + if (svc_handler->reference_counting_policy ().value () == + ACE_Event_Handler::Reference_Counting_Policy::ENABLED) + { + ref_count_guard.reset (svc_handler); + } + // Cancel the non-blocking connection. this->cancel (svc_handler); diff --git a/ACE/ace/Svc_Handler.cpp b/ACE/ace/Svc_Handler.cpp index ecdc089d1d4..af57b7fb16d 100644 --- a/ACE/ace/Svc_Handler.cpp +++ b/ACE/ace/Svc_Handler.cpp @@ -305,11 +305,7 @@ ACE_Svc_Handler<PR_ST_2, ACE_SYNCH_USE>::handle_close (ACE_HANDLE, ACE_TRACE ("ACE_Svc_Handler<PR_ST_2, ACE_SYNCH_USE>::handle_close"); if (this->reference_counting_policy ().value () == - ACE_Event_Handler::Reference_Counting_Policy::ENABLED) - { - this->remove_reference (); - } - else + ACE_Event_Handler::Reference_Counting_Policy::DISABLED) { this->destroy (); } diff --git a/ACE/tests/Bug_2609_Regression_Test.cpp b/ACE/tests/Bug_2609_Regression_Test.cpp index 734ba205acd..2c94b30cd44 100644 --- a/ACE/tests/Bug_2609_Regression_Test.cpp +++ b/ACE/tests/Bug_2609_Regression_Test.cpp @@ -64,6 +64,8 @@ public: { TEST_TRACE ("handle_close"); super::handle_close (fd, mask); + // Remove own reference which we own since creation. + this->remove_reference (); if (g_handler_deleted) { ACE_ERROR ((LM_ERROR, diff --git a/ACE/tests/Bug_2610_Regression_Test.cpp b/ACE/tests/Bug_2610_Regression_Test.cpp index 648e90bf2d2..50b3e502936 100644 --- a/ACE/tests/Bug_2610_Regression_Test.cpp +++ b/ACE/tests/Bug_2610_Regression_Test.cpp @@ -55,6 +55,8 @@ public: int handle_close (ACE_HANDLE /*fd*/, ACE_Reactor_Mask /*mask*/) { TEST_TRACE ("handle_close"); + // Remove own reference which we own since creation. + this->remove_reference (); g_semaphore.release(); return 0; } @@ -75,14 +77,18 @@ protected: int rv = super::accept_svc_handler(svc_handler); if (g_acceptor_accept_fails) { + // Remove own reference which we own since creation. + svc_handler->remove_reference (); g_semaphore.release(); return -1; } return rv; } - int activate_svc_handler (My_Svc_Handler* /*svc_handler*/) + int activate_svc_handler (My_Svc_Handler *svc_handler) { TEST_TRACE ("My_Acceptor::activate_svc_handler"); + // Remove own reference which we own since creation. + svc_handler->remove_reference (); g_semaphore.release(); return -1; } diff --git a/ACE/tests/NonBlocking_Conn_Test.cpp b/ACE/tests/NonBlocking_Conn_Test.cpp index 4cad80d5583..e1501dd5a8f 100644 --- a/ACE/tests/NonBlocking_Conn_Test.cpp +++ b/ACE/tests/NonBlocking_Conn_Test.cpp @@ -37,9 +37,10 @@ static int result = 0; Svc_Handler::Svc_Handler (bool is_ref_counted) : status_ (0), - completion_counter_ (0) + completion_counter_ (0), + is_ref_counted_ (is_ref_counted) { - if (is_ref_counted) + if (this->is_ref_counted_) { // Enable reference counting on the event handler. this->reference_counting_policy ().value ( @@ -70,8 +71,21 @@ Svc_Handler::handle_close (ACE_HANDLE handle, ACE_Reactor_Mask mask) *this->status_ = Svc_Handler::Conn_FAILED; (*this->completion_counter_)++; - return ACE_Svc_Handler<ACE_SOCK_STREAM, ACE_NULL_SYNCH>::handle_close (handle, - mask); + typedef ACE_Svc_Handler<ACE_SOCK_STREAM, ACE_NULL_SYNCH> super; + + bool const is_ref_counted = this->is_ref_counted_; + + int const res = super::handle_close (handle, + mask); + + if (is_ref_counted) + { + // If we use reference counting then remove reference + // which we own since Svc_Handler creation. + this->remove_reference (); + } + + return res; } typedef ACE_Connector<Svc_Handler, ACE_SOCK_CONNECTOR> CONNECTOR; @@ -154,14 +168,6 @@ test_connect (ACE_Reactor &reactor, { svc_handlers[i]->close (); } - else if (with_ref_counting && - complete_nonblocking_connections == Svc_Handler::NO) - { - // If we use reference counting and don't wait for connections - // then they can never succeed and we have to remove - // reference manually in order to avoid memory leaks. - svc_handlers[i]->remove_reference (); - } } delete[] svc_handlers; diff --git a/ACE/tests/NonBlocking_Conn_Test.h b/ACE/tests/NonBlocking_Conn_Test.h index a22c6c431ef..dc52f3fe694 100644 --- a/ACE/tests/NonBlocking_Conn_Test.h +++ b/ACE/tests/NonBlocking_Conn_Test.h @@ -56,6 +56,7 @@ public: Connection_Status *status_; int *completion_counter_; + bool is_ref_counted_; }; #endif /* NONBLOCKING_CONN_TEST_H */ diff --git a/ACE/tests/Process_Strategy_Test.cpp b/ACE/tests/Process_Strategy_Test.cpp index a4f0d3123ba..74cbf0bfe12 100644 --- a/ACE/tests/Process_Strategy_Test.cpp +++ b/ACE/tests/Process_Strategy_Test.cpp @@ -441,8 +441,18 @@ int Counting_Service::handle_close (ACE_HANDLE, ACE_Reactor_Mask) { - // Done with another connection. - connection_completed (); + // Count completed connections here only when the test is not in + // "process-per-connection" mode. In general, this should not be + // done here. Proper place for this is activate_svc_handler() but + // since only "process-per-connection" hooks into that function in + // other modes it's done here. The later creates a problem in + // "process-per-connection" mode since it calculates the same + // connection twice and as a result it cannot finalize gracefully. + if (OPTIONS::instance ()->concurrency_type () != Options::PROCESS) + { + // Done with another connection. + connection_completed (); + } // Call down to base class return ACE_Svc_Handler<ACE_SOCK_STREAM, ACE_NULL_SYNCH>::handle_close (); |