To close on this item - The PreDestroy order for the class hierarchy
will not be changed.
We tested the order of the PreDestroy invocations on @Interceptors
interceptors and target instances against 3 appservers: the RI, WLS,
and WebSphere (let me know if anybody else is interested to give it a
try - it's a simple EJB jar that needs to be deployed/undeployed and the
log file examined for the produced output).
All 3 of them diligently followed the rules that had been originally
defined in the EJB 3.0 spec (i.e. 7 years ago), then carried over to the
Interceptors 1.1 (and now to 1.2): the superclass interceptor method is
invoked before the one in the subclass.
Regardeing the cleanup of references in the superclass: the superclass
PreDestroy method can do its cleanup *after* the ctx.proceed call
returns (i.e. after the subclass has finished its processing), which
seems to be a good option for the cleanup.
Best,
-marina
On 2/4/13 7:05 AM, Mark Struberg wrote:
>
> I've now read through the responses and to me personally the cons were 'there might be backward compat issues' without giving a single example.
>
>
> I would be happy to have a single example of a code which would be broken. Until then I see no reason to not introduce that change.
>
>
> LieGrue,
> strub
>
>
>> ________________________________
>> From: Marina Vatkina <marina.vatkina_at_oracle.com>
>> To: users_at_ejb-spec.java.net
>> Sent: Monday, February 4, 2013 2:12 AM
>> Subject: [ejb-spec users] Re: [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>>
>> Guys,
>>
>> When I asked this questions on the EJB EG (see
>> http://java.net/projects/ejb-spec/lists/jsr345-experts/archive/2011-09/thread/1)
>> the answers split almost equally between preserving the order and making
>> the change.
>>
>> I feel very uncomfortable making a last-minute change that can break an
>> existing behavior.
>>
>> -marina
>>
>> On 2/3/13 5:48 AM, Mark Struberg wrote:
>>> Btw, @PrePassivate might be a different beast. Calling this in a different order might result in different serialisation for some cases? If so then we would need to be careful to not introduce compat issues when a <=EE6 container talks with a >= EE7 container.
>>>
>>> LieGrue,
>>> strub
>>>
>>>
>>>
>>>
>>> ----- Original Message -----
>>>> From: Pete Muir <pmuir_at_redhat.com>
>>>> To: Mark Struberg <struberg_at_yahoo.de>
>>>> Cc: "marina.vatkina_at_oracle.com" <marina.vatkina_at_oracle.com>; ejb-users <users_at_ejb-spec.java.net>
>>>> Sent: Sunday, February 3, 2013 1:56 PM
>>>> Subject: Re: [ejb-spec users] [cdi-dev] Fwd: Interceptors spec 1.2 draft is available for review
>>>>
>>>> +1
>>>>
>>>> On 2 Feb 2013, at 17:16, Mark Struberg wrote:
>>>>
>>>>>>> Unfortunately @PreDestroy in RI (GlassFish) is called in the
>>>>>> "defined" order, i.e. the same order as @PostConstruct. Bill
>>>> brought
>>>>>> this up at the beginning of EE7, but because it wasn't defined when
>>>>>> introduced, it's too late to change. So all interceptors are
>>>> invoked
>>>>>> according to the same rule.
>>>>>>> Does CDI behave differently?
>>>>>> I'm pretty sure CDI does it the way mark describes. But I woulld
>>>> have to
>>>>>> check.
>>>>> Well, at least in OpenWebBeans we do it subclass first for @PreDestroy ever
>>>> since if I remember correctly. At least since a long time now, and we never had
>>>> issues with any TCK.
>>>>> The point is: parent-class-first for any destroyal operation would limit
>>>> the functionality heavily. You e.g. can only touch fields and methods on your
>>>> current class-level but not of any parent class as they might have been
>>>> destroyed already. Thus by defining this as subclass-first you only gain
>>>> functionality but don't loose any, right? Or did I overlook anything? Today
>>>> you would need to implement the @PreDestroy only in the uppermost parent and
>>>> call a virtual method which does subclass first. But even here it would be no
>>>> problem to switch to subclass first for @PreDestroy methods as they are not
>>>> implemented in the subclasses anyway...
>>>>> So in my opinion we should define this for EE7 as this looks a perfectly
>>>> backward compatible change for me. Of course this needs a review and other
>>>> brains who check that we didn't overlook something.
>>>>> Replies to the other parts will follow tomorrow.
>>>>>
>>>>>
>>>>> LieGrue,
>>>>> strub
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>>> From: Pete Muir <pmuir_at_redhat.com>
>>>>>> To: marina.vatkina_at_oracle.com
>>>>>> Cc: Mark Struberg <struberg_at_yahoo.de>; ejb-users
>>>> <users_at_ejb-spec.java.net>
>>>>>> Sent: Saturday, February 2, 2013 11:45 AM
>>>>>> Subject: Re: [ejb-spec users] [cdi-dev] Fwd: Interceptors spec 1.2
>>>> draft is available for review
>>>>>> On 2 Feb 2013, at 03:29, Marina Vatkina wrote:
>>>>>>
>>>>>>> Hi Mark,
>>>>>>>
>>>>>>> Thank you for the review!
>>>>>>>
>>>>>>> On 1/31/13 1:41 PM, Mark Struberg wrote:
>>>>>>>> Hi Marina, Pete, other EG fellows!
>>>>>>>>
>>>>>>>> I quickly read through the spec and took some small notes. Some
>>>> points
>>>>>> maybe right, others might be wrong, hope the feedback helps nontheless
>>>> ;)
>>>>>>>> Chapter 2.1:
>>>>>>>> "Around-invoke interceptor methods, around-timeout
>>>> interceptor
>>>>>> methods, and lifecycle callback interceptor methods may be defined on
>>>> the same
>>>>>> interceptor class."
>>>>>>>> This sounds like there could be multiple of each in the same
>>>> class,
>>>>>> e.g. 2 AroundInvoke. I know this is clarified later, but maybe we can
>>>> find a
>>>>>> wording which makes this more clear from the beginning. It's also
>>>> often
>>>>>> unclear if 'on the same interceptor class' includes parent
>>>> classes or
>>>>>> not. Maybe add a link to a later paragraph where this is defined much
>>>> more in
>>>>>> detail.
>>>>>>> How about this:
>>>>>>>
>>>>>>> "An interceptor class may define an interceptor method for all
>>>>>> interposed invocations: an around-invoke interceptor method,
>>>> around-timeout
>>>>>> interceptor method, and a lifecycle callback interceptor method for
>>>> each
>>>>>> lifecycle event. See section xxx on invocation order of interceptors
>>>> with
>>>>>> superclasses."
>>>>>>> Or this"
>>>>>>>
>>>>>>> "An around-invoke interceptor method, around-timeout
>>>> interceptor
>>>>>> method, and lifecycle callback interceptor methods for different
>>>> lifecycle
>>>>>> events may be defined on the same interceptor class. See section xxx on
>>>>>> invocation order of interceptors with superclasses."
>>>>>>>> Chapter 2.2:
>>>>>>>> Maybe add that @PostConstruct lifecycle interceptors are
>>>> invoked in
>>>>>> parent-class-first order, whereas @PreDestroy interceptors are invoked
>>>> in
>>>>>> subclass-first order. Or is this defined elsewhere already? They should
>>>> follow
>>>>>> constructor invocation order respective finalize order. It might also
>>>> be ok to
>>>>>> clarify that overloaded interceptor methods in subclasses disable their
>>>>>> respective information from the parent class. Even introduce an own
>>>>>> 'Interceptor Method resolution' chapter or similar?
>>>>>>> Unfortunately @PreDestroy in RI (GlassFish) is called in the
>>>>>> "defined" order, i.e. the same order as @PostConstruct. Bill
>>>> brought
>>>>>> this up at the beginning of EE7, but because it wasn't defined when
>>>>>> introduced, it's too late to change. So all interceptors are
>>>> invoked
>>>>>> according to the same rule.
>>>>>>> Does CDI behave differently?
>>>>>> I'm pretty sure CDI does it the way mark describes. But I woulld
>>>> have to
>>>>>> check.
>>>>>>
>>>>>>>> Chapter 2.5:
>>>>>>>> "An around-invoke method must not be declared as abstract,
>>>> final
>>>>>> or static."
>>>>>>>> Why so? I can see that abstract makes no sense, buy we
>>>> don't need
>>>>>> to further proxy them, don't we? So why not final nor static? Has
>>>> this to do
>>>>>> with some EJB limitations?
>>>>>>> Yes, it was an EJB limitation when interceptors were added. Is it a
>>>> problem
>>>>>> for CDI?
>>>>>>
>>>>>> No, it's just uncessary. We could relax this.
>>>>>>
>>>>>>>> "Only one around-invoke method may be defined on a given
>>>>>> class."
>>>>>>>> Including superclasses or not? Would it be ok to have 1
>>>> @AroundInvoke
>>>>>> in the subclass and another in the parent class or max 1 for the whole
>>>> class
>>>>>> hierarchy? This is in general not always clear from the wording. I
>>>> remember
>>>>>> having asked this a few years ago and since then we support 1 per each
>>>> class in
>>>>>> the hierarchy in OWB.
>>>>>>> It's there in the ordering chapter, where I just created a
>>>> special
>>>>>> section from this bullet:
>>>>>>> "If an interceptor class itself has superclasses, the
>>>> interceptor
>>>>>> methods defined by the interceptor class’s superclasses are invoked
>>>> before the
>>>>>> interceptor method defined by the interceptor class, most general
>>>> superclass
>>>>>> first."
>>>>>>> Do you think I should add a ref to each type explicitly?
>>>>>> I think this needs to be explict in the definitions as well.
>>>>>>
>>>>>>>> Chapter 2.8:
>>>>>>>> "For example, an around-invoke interceptor method may be
>>>> defined
>>>>>> to apply only to a specific target class method invocation,.."
>>>>>>>> Why 'for example'? A lifecycle interceptor cannot be
>>>> annotated
>>>>>> to some method, only @AroundInvoke can, isn't? To be honest I
>>>> didn't get
>>>>>> the paragraph about method-level timeout interceptors. Maybe it's
>>>> just me
>>>>>> understanding the term the wrong way. Can you give me a sample of a
>>>> timeout
>>>>>> interceptor applied to a method?
>>>>>>> There can be multiple timeout methods defined via @Schedule. Each
>>>> one can
>>>>>> have its own set of interceptors (strange, but possible). I can remove
>>>> "For
>>>>>> example," if you feel it's confusing.
>>>>>>>> just formatting:
>>>>>>>> page 14 is intentionally left blank?
>>>>>>> Yes. It follows EJB spec format - each chapter starts on the right
>>>> page of
>>>>>> an open book.
>>>>>>>> page 15/ Chapter3 has a huge empty block as well.
>>>>>>> Do you mean at the top? Every chapter starts at the bottom of its
>>>> page.
>>>>>>>> Chapter 3.2:
>>>>>>>> "If an interceptor does not declare an Interceptor
>>>> annotation, it
>>>>>> could be bound to components using Interceptors annotation or a
>>>> deployment
>>>>>> descriptor file."
>>>>>>>> Does this also imply forbidding the reverse? That a class
>>>> annotated
>>>>>> with @Interceptor must not be used in @Interceptors(..) ?
>>>>>>> I'm not sure we should this should be forbidden, though ch2
>>>> says
>>>>>> "Interceptors classes may be associated with the target class
>>>> using either
>>>>>> interceptor binding (see Chapter 3) or the Interceptors annotation (see
>>>> Chapter
>>>>>> 4)."
>>>>>> I don't think it does imply the reverse.
>>>>>>
>>>>>>>> This chapter should also note that the rules for activation
>>>> such
>>>>>> @Interceptor classes might be defined in other specs (e.g. for CDI only
>>>> if
>>>>>> enabled in a beans.xml). Or we define those rules here.
>>>>>>> May be ch5 should be called "Enabling and Ordering
>>>> Interceptors"?
>>>>>> Agree with Mark. Let's make sure this extensible.
>>>>>>
>>>>>>>> Otherwise an @Interceptor which is not enabled in beans.xml
>>>> would _not_
>>>>>> be active for CDI beans but enabled for EJBs? And what happens if you
>>>> have a
>>>>>> @RequestScoped @Stateful class SomeBean?
>>>>>>> If it's enabled via @Interceptors on the @Stateful SomeBean, it
>>>> will be
>>>>>> called as all other "EJB" interceptor.
>>>>>>
>>>>>> Right, EJB on it's own doesn't support interceptor bindings.
>>>>>>
>>>>>>>> Chapter 3.4:
>>>>>>>> This chapter should generally introduce the restriction on
>>>> 'active
>>>>>> interceptors'.
>>>>>>>> This chapter defines the behavior of @Nonbinding. Does this
>>>> mean we
>>>>>> need to move this annotation from the CDI spec packages (e.g.
>>>> cdi-api.jar) to
>>>>>> the interceptor spec API jar? Or better to commons-annotations?
>>>>>>> Pete though we would, but we can't move an annotation to a
>>>> different
>>>>>> package without breaking backward compatibility, and other members of
>>>> that
>>>>>> package are CDI-specific. The version that I'm working on refers to
>>>> the CDI
>>>>>> spec on @Nonbinding rules.
>>>>>>
>>>>>> We would need to move the whole package (javax.enterprise.util) to
>>>> common
>>>>>> annotations. The other members of the package is are NOT CDI specific,
>>>> and I
>>>>>> asked the Java EE spec leads to consider moving the package, but it was
>>>> turned
>>>>>> down (not sure why exactly). We do have other specs who would like this
>>>> package
>>>>>> moved (JAX-RS).
>>>>>>
>>>>>>>> Chapter 3.4.2:
>>>>>>>> @Priority: which package is this? I guess commons-annoations,
>>>> but where
>>>>>> got this specced?
>>>>>>> In the common annotation MR
>>>>>>>
>>>>>>>> Btw, it would be good to have the magic number as float…
>>>>>>> Float is too complicated ;)
>>>>>> I think int is large enough.
>>>>>>
>>>>>>>> "If an array-valued or annotation-valued member of an
>>>> interceptor
>>>>>> binding type is not annotated with the Nonbinding annotation,
>>>> non-portable
>>>>>> behavior results."
>>>>>>>> Wasn't there a solution for the array issue over on our CDI
>>>> EG
>>>>>> list?
>>>>>>> Pete?
>>>>>> We discussed it, but there are still problems with it IIRC. We can
>>>> always add
>>>>>> this later.
>>>>>>
>>>>>>>> Chapter 4:
>>>>>>>> "Method-level target class method interceptors are invoked
>>>> in
>>>>>> addition to any default interceptors and interceptors defined for the
>>>> target
>>>>>> class (and its superclasses)."
>>>>>>>> Imo this is way too blurry. Not 'in addition' but we
>>>> should
>>>>>> instead define whether 'before' or 'after', isn't?
>>>>>>> It's in addition - see rules in 5.3 (this section is changing
>>>> places
>>>>>> with 5.4/5.5 as we speak)
>>>>>>
>>>>>> Right the order is elsewhere.
>>>>>>
>>>>>>>> We should also define the order of subclass/superclass
>>>> interceptors.
>>>>>> For superclass it also depends whether the @InterceptorBinding has
>>>> @Inherited or
>>>>>> not, don't it? Or just apply standard Java rules and let
>>>>>> 'overriding' kick in?
>>>>>>> Pete said that subclass/superclass order is the same for the CDI
>>>> and EJB
>>>>>> interceptors.
>>>>>>
>>>>>> AFAIK the order is undefined.
>>>>>>
>>>>>>>> Chapter 5.2:
>>>>>>>> "If a target class has superclasses, any interceptor
>>>> methods
>>>>>> defined on those superclasses are invoked, most general superclass
>>>> first."
>>>>>>>> This is true for all but @PreDestroy and @PrePassivate
>>>> interceptors.
>>>>>> Those must not follow ct rules but finalize rules and imo should get
>>>> invoked in
>>>>>> subclass-first order.
>>>>>>> It's correct. See above ;(
>>>>>>>> Chapter 5.4:
>>>>>>>> a.) @Priority should get a float instead of an int. It's
>>>> easier to
>>>>>> squeeze something in that way.
>>>>>>>> b.) We must clarify the relation between a priority defined in
>>>>>> beans.xml and @Priority, or should we do that in the CDI spec?
>>>>>>> In the CDI spec ;)
>>>>>> Yes. I need to do this.
>>>>>>
>>>>>>>> What about EJBs then?
>>>>>>> It said - does not apply.
>>>>>>>
>>>>>>>> Well, I only did read through it once and I guess I got a few
>>>> things
>>>>>> wrong. But at least these are a few ideas floating around in my head.
>>>> Hope it
>>>>>> helps.
>>>>>>> It does. As you can see, it's not an easy task to merge 2
>>>> specs... If
>>>>>> you have any other comments in the next couple of days, please post
>>>> them. The MR
>>>>>> specs are due for the JCP submission on the 7th.
>>>>>>> Best,
>>>>>>> -marina
>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ----- Original Message -----
>>>>>>>>> From: Pete Muir <pmuir_at_redhat.com>
>>>>>>>>> To: cdi-dev_at_lists.jboss.org
>>>>>>>>> Cc:
>>>>>>>>> Sent: Wednesday, January 30, 2013 8:39 AM
>>>>>>>>> Subject: [cdi-dev] Fwd: Interceptors spec 1.2 draft is
>>>> available
>>>>>> for review
>>>>>>>>> FYI - big improvements to the interceptors spec - much
>>>> better for
>>>>>> alignment with
>>>>>>>>> CDI.
>>>>>>>>>
>>>>>>>>> I'll update CDI, next, to take account of this.
>>>>>>>>>
>>>>>>>>> Begin forwarded message:
>>>>>>>>>
>>>>>>>>>> From: Marina Vatkina
>>>> <marina.vatkina_at_oracle.com>
>>>>>>>>>> Subject: Interceptors spec 1.2 draft is available for
>>>> review
>>>>>>>>>> Date: 29 January 2013 22:06:36 GMT
>>>>>>>>>> To: jsr345-experts_at_ejb-spec.java.net, Pete Muir
>>>>>> <pmuir_at_redhat.com>
>>>>>>>>>> After a lot of work by Linda, me, and Pete, we have
>>>> the
>>>>>> Interceptors spec
>>>>>>>>> 1.2 draft for review:
>>>>>>>>>
>>>> http://java.net/projects/interceptors-spec/downloads/download/interceptor-1-2-dr1.pdf
>>>>>>>>>> What's there:
>>>>>>>>>> Editorial cleanup and conversion to standard
>>>> template.
>>>>>>>>>> Assigned chapter numbers to sections and rearranged
>>>> various
>>>>>> sections and
>>>>>>>>> examples for better flow.
>>>>>>>>>> Clarified statement regarding transaction context of
>>>> lifecycle
>>>>>> callback
>>>>>>>>> methods
>>>>>>>>>> Added a note on a timeout method that is also a
>>>> business
>>>>>> method and
>>>>>>>>> around-timeout and around-invoke interceptors
>>>>>>>>>> Added Chapter 1 (Overview)
>>>>>>>>>> Added Chapter 3, derived from Chapter 9 of the CDI
>>>>>> specification.
>>>>>>>>>> Removed deployment descriptors definitions (general
>>>> notes
>>>>>> about possibility
>>>>>>>>> of DDs are there)
>>>>>>>>>> Added examples with interceptor bindings to common
>>>> sections.
>>>>>>>>>> Added standard Priority ranges
>>>>>>>>>>
>>>>>>>>>> What's not there:
>>>>>>>>>> @AroundConstruct interceptor
>>>>>>>>>> Notes on "throws Exception" clauses in the
>>>>>> around-xxx method
>>>>>>>>> signatures
>>>>>>>>>> Perfect CDI alignment - the text (and fonts) might be
>>>> not
>>>>>> fully aligned.
>>>>>>>>>> Please review ASAP.
>>>>>>>>>>
>>>>>>>>>> Pete, please forward to the CDI EG.
>>>>>>>>>>
>>>>>>>>>> thanks,
>>>>>>>>>> -marina
>>>>>>>>> _______________________________________________
>>>>>>>>> cdi-dev mailing list
>>>>>>>>> cdi-dev_at_lists.jboss.org
>>>>>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>>>>>>>
>>
>>
>>