diff options
| -rw-r--r-- | TAO/ChangeLog | 62 | ||||
| -rw-r--r-- | TAO/tao/BiDir_Adapter.h | 2 | ||||
| -rw-r--r-- | TAO/tao/BiDir_GIOP/BiDirGIOP.cpp | 58 | ||||
| -rw-r--r-- | TAO/tao/BiDir_GIOP/BiDirGIOP.h | 12 | ||||
| -rw-r--r-- | TAO/tao/ORB_Core.cpp | 5 | ||||
| -rw-r--r-- | TAO/tao/ORB_Core.h | 3 | ||||
| -rw-r--r-- | TAO/tao/Policy_Validator.cpp | 6 | ||||
| -rw-r--r-- | TAO/tao/Policy_Validator.h | 5 | ||||
| -rw-r--r-- | TAO/tao/PortableServer/POA_Policy_Set.cpp | 2 | ||||
| -rw-r--r-- | TAO/tests/BiDirectional_MultipleORB/README | 23 | ||||
| -rw-r--r-- | TAO/tests/BiDirectional_MultipleORB/destroy.cpp | 123 | ||||
| -rw-r--r-- | TAO/tests/BiDirectional_MultipleORB/destroy.mpc | 6 | ||||
| -rwxr-xr-x | TAO/tests/BiDirectional_MultipleORB/run_test.pl | 21 |
13 files changed, 277 insertions, 51 deletions
diff --git a/TAO/ChangeLog b/TAO/ChangeLog index a78fe567445..e3da9e7271a 100644 --- a/TAO/ChangeLog +++ b/TAO/ChangeLog @@ -1,8 +1,66 @@ +Mon Dec 6 16:00:46 2004 Iliyan Jeliazkov <jeliazkov_i@ociweb.com> + + This change solves a problem with the bi-dir policy + validators, in the context of multiple ORBs by + eliminating cached, ORB-specific state in the + TAO_BiDirGIOP_Loader singleton. Thus it also + eliminates the possibility for a race condition + between two threads that my be creating and + destroying ORB instances simultaneously. + + * tao/BiDir_Adapter.h: + + Changed the method decl to support passing up any exceptions for + platforms that do not support them natively. + + * tao/BiDir_GIOP/BiDirGIOP.h: + * tao/BiDir_GIOP/BiDirGIOP.cpp: + + Removed state, represented by the bi-dir policy validator + instance, which was carried around in the Loader even + after the corresponding ORB was destructed. This caused + problems in processes with >1 ORB, where the new ORB was + re-using an validator instance, associated with another ORB. + In certain cases that validator was also already deleted. + Now a bi-dir policy validator gets instantiated and + registered during the call to + TAO_BiDirGIOP_Loader::load_policy_validators, + instead of piggy-backing on register_orb_initializer and + caching it. + + * tao/ORB_Core.h: + * tao/ORB_Core.cpp: + * tao/Policy_Validator.h: + * tao/Policy_Validator.cpp: + + Provided accessor for the ORB reference to enable the + bi-dir policy validator instantiation in + TAO_BiDirGIOP_Loader::load_policy_validators + + * tao/PortableServer/POA_Policy_Set.cpp: + + Changed the invocation of the method to pass up any exceptions on + platforms that do not support them natively. + + * tests/BiDirectional_MultipleORB/README: + * tests/BiDirectional_MultipleORB/destroy.mpc: + * tests/BiDirectional_MultipleORB/destroy.cpp: + * tests/BiDirectional_MultipleORB/run_test.pl: + + This is a test that creates a bidir GIOP policy + for a POA and then attempts to repeat this, after first + destructing and re-creating the ORB. This used to fail + by breaking an assertion, because the second ORB was + tryig to register a policy validator object instance, + which lingered since the time the first ORB was created. + In some cases it broke with SEGV, as when trying to access + a previously deleted bi-dir policy validator. + Mon Dec 6 13:14:32 2004 Jeff Parsons <j.parsons@vanderbilt.edu> * orbsvcs/IFR_Service/IFR_Service.mpc: - - Removed inherited .mpb files iortable, svc_utils, and + + Removed inherited .mpb files iortable, svc_utils, and typecodefactory, since they are already pulled in by ifrservice. Also added empty IDL_Files block. Because of all the inherited .mpb files, any IDL file in the diff --git a/TAO/tao/BiDir_Adapter.h b/TAO/tao/BiDir_Adapter.h index f28ca8f2107..53cdcaafd96 100644 --- a/TAO/tao/BiDir_Adapter.h +++ b/TAO/tao/BiDir_Adapter.h @@ -52,7 +52,7 @@ public: ACE_ENV_ARG_DECL) ACE_THROW_SPEC ((CORBA::SystemException)) = 0; - virtual void load_policy_validators (TAO_Policy_Validator &validator) + virtual void load_policy_validators (TAO_Policy_Validator &validator ACE_ENV_ARG_DECL) ACE_THROW_SPEC ((CORBA::SystemException)) = 0; }; diff --git a/TAO/tao/BiDir_GIOP/BiDirGIOP.cpp b/TAO/tao/BiDir_GIOP/BiDirGIOP.cpp index 945b3e00b14..f962c57ed50 100644 --- a/TAO/tao/BiDir_GIOP/BiDirGIOP.cpp +++ b/TAO/tao/BiDir_GIOP/BiDirGIOP.cpp @@ -11,18 +11,14 @@ ACE_RCSID (BiDir_GIOP, // Set the flag to zero to start with -int TAO_BiDirGIOP_Loader::validator_loaded_ = 0; int TAO_BiDirGIOP_Loader::is_activated_ = 0; TAO_BiDirGIOP_Loader::TAO_BiDirGIOP_Loader (void) - : validator_ (0) { } TAO_BiDirGIOP_Loader::~TAO_BiDirGIOP_Loader (void) { - /* if (this->validator_) - delete this->validator_;*/ } int @@ -32,6 +28,8 @@ TAO_BiDirGIOP_Loader::activate (CORBA::ORB_ptr orb, ACE_ENV_ARG_DECL) ACE_THROW_SPEC ((CORBA::SystemException)) { + ACE_UNUSED_ARG (orb); + if (TAO_BiDirGIOP_Loader::is_activated_ == 0 && TAO_DEF_GIOP_MINOR >= 2) { PortableInterceptor::ORBInitializer_ptr tmp_orb_initializer = @@ -55,19 +53,6 @@ TAO_BiDirGIOP_Loader::activate (CORBA::ORB_ptr orb, ACE_ENV_ARG_PARAMETER); ACE_CHECK_RETURN (-1); - TAO_ORB_Core *orb_core = - orb->orb_core (); - - ACE_NEW_THROW_EX (this->validator_, - TAO_BiDirPolicy_Validator (*orb_core), - CORBA::NO_MEMORY ( - CORBA::SystemException::_tao_minor_code ( - TAO_DEFAULT_MINOR_CODE, - ENOMEM), - CORBA::COMPLETED_NO)); - ACE_CHECK_RETURN (-1); - - TAO_BiDirGIOP_Loader::is_activated_ = 1; } @@ -78,9 +63,27 @@ void TAO_BiDirGIOP_Loader::load_policy_validators (TAO_Policy_Validator &val) ACE_THROW_SPEC ((CORBA::SystemException)) { - // Add our validator - if (!validator_loaded_) - val.add_validator (this->validator_); + // Is this true? Does the GIOP protocol version matter here? + if (TAO_DEF_GIOP_MINOR < 2) + return; + + TAO_BiDirPolicy_Validator *validator = 0; + ACE_NEW_THROW_EX (validator, + TAO_BiDirPolicy_Validator (val.orb_core ()), + CORBA::NO_MEMORY ( + CORBA::SystemException::_tao_minor_code ( + TAO_DEFAULT_MINOR_CODE, + ENOMEM), + CORBA::COMPLETED_NO)); + ACE_CHECK; + + // We may be adding another TAO_BiDirPolicy_Validator instance for the + // same ORB (different POA). In cases where huge numbers of bi-directional POA instances + // are created, having a validator instance per POA may introduce additional delays in + // policy validation and hence, the overal policy creation time. Since this is out of the + // critical invocation processing path, I plan to keep the design simple and not try to + // avoid an ineficiency of such small proportions. + val.add_validator (validator); } int @@ -89,24 +92,11 @@ TAO_BiDirGIOP_Loader::Initializer (void) return ACE_Service_Config::process_directive (ace_svc_desc_TAO_BiDirGIOP_Loader); } -/*static */ int -TAO_BiDirGIOP_Loader::validator_loaded (void) -{ - return validator_loaded_; -} - -/*static */ void -TAO_BiDirGIOP_Loader::validator_loaded (int f) -{ - // @@ TODO: Do we need synchronization? - validator_loaded_ = f; -} - - ACE_STATIC_SVC_DEFINE (TAO_BiDirGIOP_Loader, ACE_TEXT ("BiDirGIOP_Loader"), ACE_SVC_OBJ_T, &ACE_SVC_NAME (TAO_BiDirGIOP_Loader), ACE_Service_Type::DELETE_THIS | ACE_Service_Type::DELETE_OBJ, 0) + ACE_FACTORY_DEFINE (TAO_BiDirGIOP, TAO_BiDirGIOP_Loader) diff --git a/TAO/tao/BiDir_GIOP/BiDirGIOP.h b/TAO/tao/BiDir_GIOP/BiDirGIOP.h index 8fa5d711a18..2d5fff19ff7 100644 --- a/TAO/tao/BiDir_GIOP/BiDirGIOP.h +++ b/TAO/tao/BiDir_GIOP/BiDirGIOP.h @@ -50,23 +50,15 @@ public: ACE_ENV_ARG_DECL) ACE_THROW_SPEC ((CORBA::SystemException)); - virtual void load_policy_validators (TAO_Policy_Validator &validator) + virtual void load_policy_validators (TAO_Policy_Validator &validator + ACE_ENV_ARG_DECL) ACE_THROW_SPEC ((CORBA::SystemException)); /// Used to force the initialization of the ORB code. static int Initializer (void); - /// Accessor for the <validator_loaded_> flag - static int validator_loaded (void); - static void validator_loaded (int f); private: - /// Our policy validator - TAO_BiDirPolicy_Validator *validator_; - - /// Flag to indicate whether validator has been loaded - static int validator_loaded_; - /// Flag to indicate whether the BiDirGIOP library has been /// activated. static int is_activated_; diff --git a/TAO/tao/ORB_Core.cpp b/TAO/tao/ORB_Core.cpp index 46be618d904..2512e95d2c5 100644 --- a/TAO/tao/ORB_Core.cpp +++ b/TAO/tao/ORB_Core.cpp @@ -1661,11 +1661,12 @@ TAO_ORB_Core::create_stub_object (TAO_MProfile &mprofile, } void -TAO_ORB_Core::load_policy_validators (TAO_Policy_Validator &validator) +TAO_ORB_Core::load_policy_validators (TAO_Policy_Validator &validator + ACE_ENV_ARG_DECL) { // Call the BiDir library if it has been loaded if (this->bidir_adapter_) - this->bidir_adapter_->load_policy_validators (validator); + this->bidir_adapter_->load_policy_validators (validator ACE_ENV_ARG_PARAMETER); } CORBA::Object_ptr diff --git a/TAO/tao/ORB_Core.h b/TAO/tao/ORB_Core.h index 54330c25851..3fa71e145a6 100644 --- a/TAO/tao/ORB_Core.h +++ b/TAO/tao/ORB_Core.h @@ -963,7 +963,8 @@ public: /// Call the libraries to handover the validators if they havent /// registered yet with the list of validators. - void load_policy_validators (TAO_Policy_Validator &validator); + void load_policy_validators (TAO_Policy_Validator &validator + ACE_ENV_ARG_DECL); /// Return the flushing strategy /** diff --git a/TAO/tao/Policy_Validator.cpp b/TAO/tao/Policy_Validator.cpp index 4e78e8223db..cd51a8a1426 100644 --- a/TAO/tao/Policy_Validator.cpp +++ b/TAO/tao/Policy_Validator.cpp @@ -25,6 +25,12 @@ TAO_Policy_Validator::~TAO_Policy_Validator (void) } } +TAO_ORB_Core & +TAO_Policy_Validator::orb_core() const +{ + return this->orb_core_; +} + void TAO_Policy_Validator::add_validator (TAO_Policy_Validator *validator) { diff --git a/TAO/tao/Policy_Validator.h b/TAO/tao/Policy_Validator.h index 52f91e4847f..2c1afd96800 100644 --- a/TAO/tao/Policy_Validator.h +++ b/TAO/tao/Policy_Validator.h @@ -89,6 +89,11 @@ public: void add_validator (TAO_Policy_Validator *validator); + /** + * Accessor for the stored ORB core reference + */ + TAO_ORB_Core & orb_core() const; + protected: virtual void validate_impl (TAO_Policy_Set &policies ACE_ENV_ARG_DECL) = 0; diff --git a/TAO/tao/PortableServer/POA_Policy_Set.cpp b/TAO/tao/PortableServer/POA_Policy_Set.cpp index 9907212b534..0c37b9d5535 100644 --- a/TAO/tao/PortableServer/POA_Policy_Set.cpp +++ b/TAO/tao/PortableServer/POA_Policy_Set.cpp @@ -60,7 +60,7 @@ TAO_POA_Policy_Set::validate_policies (TAO_Policy_Validator &validator, { // Just give a last chance for all the unloaded validators in other // libraries to be registered - orb_core.load_policy_validators (validator); + orb_core.load_policy_validators (validator ACE_ENV_ARG_PARAMETER); // Validate that all of the specified policies make sense. validator.validate (this->impl_ ACE_ENV_ARG_PARAMETER); diff --git a/TAO/tests/BiDirectional_MultipleORB/README b/TAO/tests/BiDirectional_MultipleORB/README new file mode 100644 index 00000000000..2d26bcc817e --- /dev/null +++ b/TAO/tests/BiDirectional_MultipleORB/README @@ -0,0 +1,23 @@ +# $Id$ + +This is a test that creates a birectional GIOP policy for a POA and then +attempts to repeat this, after destructing and re-creating the ORB. This used to +fail by breaking an assertion, because the second ORB was tryig to register a policy +validator object instance, which lingered since the time the first ORB was created. + +The validators are chained in a linked list, and any attempt to register a new one, +which already points to another, is considered an error: + +ACE_ASSERT: (24189|4143901376) file Policy_Validator.cpp, line 28 assertion failed for 'validator->next_ == 0'.Aborting... + +In other instances it broke with SEGV, when trying to access an previously deleted +bi-dir policy validator. + +For additional detail reffer to RT4667 and RT4672. + +Start the test like this: + +$ destroy + +It should complete ok + diff --git a/TAO/tests/BiDirectional_MultipleORB/destroy.cpp b/TAO/tests/BiDirectional_MultipleORB/destroy.cpp new file mode 100644 index 00000000000..8cdf84b2401 --- /dev/null +++ b/TAO/tests/BiDirectional_MultipleORB/destroy.cpp @@ -0,0 +1,123 @@ +// $Id$ + +//======================================================================== +// +// = LIBRARY +// TAO/tests/BiDir_Multiple_ORB +// +// = FILENAME +// destroy.cpp +// +// = DESCRIPTION +// Modified ORB destruction test. +// +// = AUTHOR +// Andrew Schnable <Andrew.Schnable@veritas.com> +// Iliyan Jeliazkov <jeliazkov_i@ociweb.com> +// +//========================================================================= + +#include "tao/corba.h" +#include "tao/PortableServer/PortableServer.h" +#include "tao/BiDir_GIOP/BiDirGIOP.h" + +ACE_RCSID(BiDir_Multiple_ORB, destroy, "$Id$") + +int +test_with_bidir_poa (int argc, + char **argv, + const char *orb_name, + int destroy_orb) +{ + ACE_DECLARE_NEW_CORBA_ENV; + + ACE_TRY + { + CORBA::ORB_var orb = + CORBA::ORB_init (argc, argv, orb_name ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + CORBA::Object_var obj = + orb->resolve_initial_references ("RootPOA" + ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + PortableServer::POA_var root_poa = + PortableServer::POA::_narrow (obj.in () ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + PortableServer::POAManager_var poa_manager = + root_poa->the_POAManager (ACE_ENV_SINGLE_ARG_PARAMETER); + + ACE_TRY_CHECK; + + CORBA::PolicyList policies (1); + policies.length (1); + + CORBA::Any pol; + pol <<= BiDirPolicy::BOTH; + policies[0] = + orb->create_policy (BiDirPolicy::BIDIRECTIONAL_POLICY_TYPE, + pol + ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + // Create POA as child of RootPOA with the above policies. This POA + // will receive request in the same connection in which it sent + // the request + PortableServer::POA_var child_poa = + root_poa->create_POA ("childPOA", + poa_manager.in (), + policies + ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + // Creation of childPOA is over. Destroy the Policy objects. + for (CORBA::ULong i = 0; + i < policies.length (); + ++i) + { + policies[i]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER); + ACE_TRY_CHECK; + } + + poa_manager->activate (ACE_ENV_SINGLE_ARG_PARAMETER); + ACE_TRY_CHECK; + + + root_poa->destroy (1, 1 ACE_ENV_ARG_PARAMETER); + ACE_TRY_CHECK; + + if (destroy_orb) + { + orb->destroy (ACE_ENV_SINGLE_ARG_PARAMETER); + ACE_TRY_CHECK; + } + } + ACE_CATCHANY + { + ACE_PRINT_EXCEPTION (ACE_ANY_EXCEPTION, + "Exception raised"); + ACE_CHECK_RETURN (-1); + } + ACE_ENDTRY; + + return 0; +} + +int +main (int argc, char **argv) +{ + int result = 0; + + for (int i=0; i<10; i++) + { + result = test_with_bidir_poa (argc, argv, "poa_1", 1); + ACE_ASSERT (result == 0); + + result = test_with_bidir_poa (argc, argv, "poa_2", 1); + ACE_ASSERT (result == 0); + } + ACE_DEBUG ((LM_DEBUG, "Completed OK\n")); + return result; +} diff --git a/TAO/tests/BiDirectional_MultipleORB/destroy.mpc b/TAO/tests/BiDirectional_MultipleORB/destroy.mpc new file mode 100644 index 00000000000..1886f7e76cc --- /dev/null +++ b/TAO/tests/BiDirectional_MultipleORB/destroy.mpc @@ -0,0 +1,6 @@ +project: taoexe, portableserver, bidir_giop { + Source_Files { + destroy.cpp + } +} + diff --git a/TAO/tests/BiDirectional_MultipleORB/run_test.pl b/TAO/tests/BiDirectional_MultipleORB/run_test.pl new file mode 100755 index 00000000000..cf56e4473d7 --- /dev/null +++ b/TAO/tests/BiDirectional_MultipleORB/run_test.pl @@ -0,0 +1,21 @@ +eval '(exit $?0)' && eval 'exec perl -S $0 ${1+"$@"}' + & eval 'exec perl -S $0 $argv:q' + if 0; + +# $Id$ +# -*- perl -*- + +use lib '../../../bin'; +use PerlACE::Run_Test; + +$status = 0; +$CL = new PerlACE::Process ("destroy"); + +$client = $CL->SpawnWaitKill (10); + +if ($client != 0) { + print STDERR "ERROR: client returned $client\n"; + $status = 1; +} + +exit $status; |