Please report new issues athttps://github.com/DOCGroup
If ServantLocator::preinvoke() throws Exceptions, postinvoke() must not be called according to the COARBA-2.6 spec, $11.3.6. With TAO-1.5, postinvoke is called in this case, with parameter operation set to 0. Even worse IMHO (although the spec is not very clear about this), if preinvoke() returns 0, meaning no servant was found, postinvoke() is called as well. My investigation revealed the following: TAO_Object_Adapter::dispatch_servant() creates a Servant_Upcall instance and calls Servant_Upcall::prepare_for_upcall() This method finally calls RequestProcessingStrategyServantLocator::locate_servant() , where the follwing happens in this order: - Servant_Upcall::state (OBJECT_ADAPTER_LOCK_RELEASED); - ServantLocator::preinvoke() - Servant_Upcall::operation(operation) if ServantLocator::preinvoke() throws an Execption, the destructor of Servant_Upcall is called when the stack is unwound, and this destructor calls Servant_Upcall::upcall_cleanup(). This method contains a switch on the member state_. As state_ has been set to OBJECT_ADAPTER_LOCK_RELEASED (see above), ServantLocator::postinvoke() is now called with NULL operation. The case when ServantLocator::preinvoke() returns NULL is very similar... My quick hack to fix this problem: In Servant_Upcall::upcall_cleanup(), call ServantLocator::postinvoke() only if operation_ and servant_ are both not NULL. --------------------- snip snip ----------------------- diff -r -u PortableServer.orig/Servant_Upcall.cpp PortableServer/Servant_Upcall.cpp --- PortableServer.orig/Servant_Upcall.cpp 2006-02-13 04:23:52.000000000 +0100 +++ PortableServer/Servant_Upcall.cpp 2006-04-21 11:00:18.480507500 +0200 @@ -288,7 +288,10 @@ // Cleanup servant locator related state. Note that because // this operation does not change any Object Adapter related // state, it is ok to call it outside the lock. - this->post_invoke_servant_cleanup (); + if ( this->operation() && this->servant() ) + { + this->post_invoke_servant_cleanup (); + } // Since the object adapter lock was released, we must acquire // it. --------------------- snip snip -----------------------
Created attachment 529 [details] patch created by diff that fixes the bug
to normal pool
can you provide also a test program for this one, makes it easier to analyze and makes sure things don't get introduced back in the future
Created attachment 534 [details] Test case the exposes the bug
I will add the regression to cvs soon
accept, test is under tests/POA
added dependency on another postinvoke bug
added blocks
Wed Feb 21 12:23:12 UTC 2007 Johnny Willemsen <jwillemsen@remedy.nl> * tests/POA/Bug_2511_Regression/server.cpp: Return the error count, in case of a failure then the test framework will notice this * tao/PortableServer/RequestProcessingStrategyServantLocator.cpp: Don't call postinvoke when we don't have a servant. Fixes bugzilla bugs 2511 and 2658. Thanks to Martin Cornelius <Martin dot Cornelius at smiths-heimann dot com> and Milan Cvetkovic <milan dot cvetkovic at mpathix dot com> for reporting this and providing a test case and a proposed fix.
fixed