This shall be implemented as part of the work in implementing
Portable Interceptors according to the Joint Revision specification.

Is this fixed finally?  Can we wipe this off the list?

If it was fixed, I would have marked it as such.  It shouldn't be hard to add
support for this in the stubs and skeletons.  Perhaps our ThruPOA expert can
work on this bug.

Reassigning to tao-support since I don't have time to work on this bug.

I've looked into this bug a bit more.  It's not as simple as I thought to
implement interceptor support for the THRU_POA case.

The problem is that TAO's implementation of the
PortableInterceptor::ClientRequestInfo and
PortableInterceptor::ServerRequestInfo objects rely on the TAO_GIOP_Invocation
and TAO_ServerRequest objects, respectively.  However, neither is used in the
THRU_POA case.

The TAO_GIOP_Invocation and TAO_ServerRequest objects are necessary to support
the PortableInterceptor::ForwardRequest exception, and many of the methods
exported by each of the RequestInfo objects.  It is possible to instantiate the
TAO_GIOP_Invocation and TAO_ServerRequest objects in the THRU_POA proxy
implementation.  However, that seems like a bit of a hack.

Some other things to consider are the following:

- both client and server request interceptors must be invoked in the THRU_POA
proxy implementation.
- since both must be invoked in the proxy implementation, care must be taken to
invoke the appropriate interception point(s) in the event an exception occurs.

Here's a basic idea of the implementation necessary to implement portable
interceptors for the THRU_POA case:

SOME_RETURN_TYPE
POA_foo::_TAO_Policy_ThruPOA_Proxy_Impl::foo (
    TAO_Remote_Object *_collocated_tao_target_,
    CORBA::Environment &ACE_TRY_ENV
  )
  ACE_THROW_SPEC ((
    CORBA::SystemException
  ))
{
  SOME_RETURN_TYPE r = 0;

  TAO_Object_Adapter::Servant_Upcall servant_upcall (
      _collocated_tao_target_->_stubobj ()->servant_orb_var ()->orb_core ()
    );
  CORBA::Object_var forward_to;
  servant_upcall.prepare_for_upcall (
      _collocated_tao_target_->_object_key (),
      "foo",
      forward_to.out (),
      ACE_TRY_ENV
    );
      ACE_CHECK_RETURN (r);

#if TAO_HAS_INTERCEPTORS == 1

      TAO_ClientRequestInterceptor_Adapter cra (
        stub->orb_core ()->client_request_interceptors (),
        &_tao_call,
        _invoke_status
        );

      // Operation-specific PortableInterceptor::ClientRequestInfo.
      TAO_ClientRequestInfo_foo_foo_foo cri (
        &_tao_call,
        _collocated_tao_target_,
        ACE_TRY_ENV
      );
      ACE_CHECK;

      TAO_ServerRequestInterceptor_Adapter sra (
        _tao_server_request.orb_core ()->server_request_interceptors (),
        _tao_server_request.interceptor_count ()
        );

      // Operation-specific PortableInterceptor::ServerRequestInfo.
      TAO_ServerRequestInfo_foo_foo_foo sri (
        _tao_server_request,
        _tao_upcall,
        _tao_impl,
        ACE_TRY_ENV
        );

      PortableInterceptor::ReplyStatus status;

      ACE_TRY
        {
          // @@ The following three interception points can issue
          //    LOCATION_FORWARDs.  How do we handle them for this
          //    THRU_POA case.

          // Invoke the client request starting interception point.
          cra.send_request (&cri, ACE_TRY_ENV);
          ACE_TRY_CHECK;

          // Invoke the server request starting interception point.
          sra.receive_request_service_contexts (&sri, ACE_TRY_ENV);
          ACE_TRY_CHECK;

          // Invoke the server request intermediate interception
          // point.
          sra.receive_request (&sri, ACE_TRY_ENV);
          

#endif  /* TAO_HAS_INTERCEPTORS == 1 */

          r =
            ACE_reinterpret_cast (
              APPROPRIATE_OBJECT_TYPE,
              servant_upcall.servant ()->_downcast (
                                            "IDL:APPROPRIATE_INTF_REPO_ID"
                                            )
            )->foo (ACE_TRY_ENV);
          TAO_INTERCEPTOR_CHECK_RETURN (r);

#if TAO_HAS_INTERCEPTORS == 1
          cri.result (r);
          sri.result (r);

          // Invoke the server ending interception point
          sra.send_reply (&sri, ACE_TRY_ENV);
          ACE_TRY_CHECK;

          // Invoke the client ending interception point
          cra.receive_reply (&cri, ACE_TRY_ENV);
          ACE_TRY_CHECK; 
        }
      ACE_CATCHANY
        {
          // @@ This isn't exception safe.  send_exception() could
          //    throw an exception, itself.  In that event,
          //    receive_exception() exception point would never be
          //    invoked, which breaks the request interceptor flow.
          //    We could wrap the send_exception() call inside another
          //    ACE_TRY/CATCH block but that seems really ugly.
          //    Another option could be to use the "guard" idiom we
          //    use for mutexes.  It would make sure the all
          //    exception related ending interception points are
          //    invoked in the event of an exception.  Food for
          //    thought.

          // Invoke the server ending interception point
          sri.exception (&ACE_ANY_EXCEPTION);
          sra.send_exception (&sri, ACE_TRY_ENV);
          ACE_TRY_CHECK;

          status =
            sri.reply_status (ACE_TRY_ENV);
          ACE_TRY_CHECK;

          // Invoke the appropriate client ending interception point
          if (status == PortableInterceptor::LOCATION_FORWARD)
            {
              // send_exception() can potentially throw a
              // PortableInterceptor::ForwardRequest exception.  In
              // that case, we need to invoke a the receive_other()
              // interception point.
              // @@ TODO: How do we handle LOCATION_FORWARDs initiated
              //          by an interception point for this THRU_POA
              //          case.
              cra.receive_other (&cri, ACE_TRY_ENV);
              ACE_TRY_CHECK;

              return r;
            }
          else
            {
              // @@ If send_exception() transforms the exception, then
              //    the &ACE_ANY_EXCEPTION below is incorrect!  FIXME!
              cri.exception (&ACE_ANY_EXCEPTION);
              cra.receive_exception (&cri, ACE_TRY_ENV);
              ACE_TRY_CHECK;

              status = cri.reply_status (ACE_TRY_ENV);
              ACE_TRY_CHECK;
            }

          // Only re-throw the exception if the
          // send/receive_exception() interception points did not
          // transform it to a LOCATION_FORWARD, for example.
          if (status == PortableInterceptor::SYSTEM_EXCEPTION
              || status == PortableInterceptor::USER_EXCEPTION)
            ACE_RE_THROW;
        }
      ACE_ENDTRY;
      ACE_CHECK_RETURN (0);
#endif  /* TAO_HAS_INTERCEPTORS == 1 */

  return r;
}

BTW, I am not saying that we use the above code as the implementation.  I was
just building on the way we currently implement interceptors, which isn't
necessarily the best way of doing things.

Long time back we had a conversation about using Decorators to implement the
interception points.  Basically what we could do is decorate the Proxy object
with an interceptor.  If the proxy is remote or thru POA the interceptors would
still work.

Here is my original email:

 we
should use the proxy brokers and decorators to implement
interceptors.  Of course I'm making no sense now, but I think we
should explore this idea.  Basically the trick is to implement proxies
like this:

// IDL
interface Foo { void an_operation (in long x); };

// C++

class TAO_Foo_Interceptor_Proxy_Impl : public TAO_Foo_Proxy_Impl
{
public:
  /// Created by the Proxy_broker when interceptors are enabled,
  /// otherwise the 'regular_proxy' is returned.
  TAO_Foo_Interceptor_Proxy_Impl (TAO_Foo_Proxy_Impl *proxy)
    : proxy_ (proxy) {}

  void an_operation (CORBA_Object *target,
		     CORBA::Long an_argument)
  {
     Foo_ClientRequestInfo ri ("an_operation",
			   service_context_list, // @@
			   target->stub (),
			   );
     // Invoke some interceptors here...    

    try
    {
      // Now invoke the real proxy (collocated or remote!)
      real_proxy->an_operation (object, an_argument);

      // Invoke the receive reply interception point
    }
    catch (CORBA::Exception &ex)
    {
      // Invoke the exception interception point
      rethrow;
    }
  }
};

	If we could pull this one off we would have the following
benefits:

- Interceptors would work for local and remote calls.
- Interceptors would be a *dynamic* feature: less compile-time
  dependencies, zero overhead until real interceptors are installed.
- Interceptors could be compiled in a separate file (say FooC_I.cpp):
  less footprint in services like naming or trading.

       I know there are a ton of problems with this approach, for
example, the current interceptors grab the service context list from
the Invocation object, but that must be changed anyways if the
interceptors are to be supported in collocated calls.  For example, we
could reverse the role, and have the interceptor allocate the service
context list.  There is also a problem with interception points,
specially getting the request id.  Maybe breaking the invocation
routine in pieces could take care of that..

I believe that once bug 1369 is fixed it would be much simpler to fix this problem.

*** Bug 1868 has been marked as a duplicate of this bug. ***

Why not as long as this bug is not fixed disable the following tests in our 
daily builds and enable them again when this item is fixed? That way for some 
builds the test column gets green and we can easier see when things break that 
now work.

TAO/tests/Portable_Interceptors/Collocated/Dynamic
TAO/tests/Portable_Interceptors/Collocated/Service_Context_Manipulation

Bala, didn't you and Jeff add thru POA collocation interceptor support in your
overhaul last year?

Nevermind. I forgot that you addressed the client side interception points not
the server side.

The skeleton refactoring changes described in Bug 1369 that were recently merged
to the HEAD branch fix this problem.

reopen again, regression tests show we still have a small problem. Will work on 
this today

reassign

updated version

fixed now