dev@glassfish.java.net

Re: regression on dynamic reconfig

From: Amy Roh <Amelia.Roh_at_Sun.COM>
Date: Tue, 16 Sep 2008 22:57:53 -0700

Jerome Dochez wrote:
> Amy Roh wrote:
>
>> Jerome Dochez wrote:
>>
>>> indeed I can see there is a problem :(
>>> however, I think your fix might still not be complete...
>>>
>>> can you try mine :)
>>
>>
>> Dynamic reconfig tests like your patch too. :-)
>>
>> Can we apply the attached patch before next integration also? It
>> should address hk2.config.Transactions NPE which happens at times.
>>
> I am checking it in right now, then

Thanks!

Hmm I see you already pushed out 0.3.23. There was a second patch on
hk2.config.Transactions attached to the previous email which resolves
another NPE issue. I was hoping to check that in before the
integration. Can you check it in if you approve?

Thanks,
Amy

>
> thanks for testing....
>
> jerome
>
>> Thanks,
>> Amy
>>
>>
>>> thanks, jerome
>>>
>>> Index: config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> ===================================================================
>>> --- config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> (revision 839)
>>> +++ config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>> (working copy)
>>> @@ -292,8 +292,8 @@
>>> // we don't really send the changed events for
>>> new comers.
>>> continue;
>>> }
>>> + ConfigBeanProxy proxy = null;
>>> if (event.getNewValue()==null) {
>>> - ConfigBeanProxy proxy = null;
>>> try {
>>> // getOldValue() can be null, we will notify
>>> a CHANGE event
>>> proxy =
>>> ConfigBeanProxy.class.cast(event.getOldValue());
>>> @@ -301,6 +301,7 @@
>>> // this is ok, the old value was probably a
>>> string or something like this...
>>> // we will notify the event.getSource() that
>>> it changed.
>>> }
>>> + // new value is null, but not old value, we
>>> removed something
>>> if (proxy!=null) {
>>> final NotProcessed nc =
>>> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
>>> if ( nc != null ) {
>>> @@ -308,13 +309,17 @@
>>> }
>>> continue;
>>> }
>>> - if (!changed.contains(eventSource)) {
>>> - proxy =
>>> ConfigBeanProxy.class.cast(event.getSource());
>>> - changed.add(eventSource);
>>> - final NotProcessed nc =
>>> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
>>> - if ( nc != null ) {
>>> - unprocessed.add( new
>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>> - }
>>> + }
>>> + // we fall back in this case, the newValue was nullm
>>> the old value was also null or was not a ConfigBean,
>>> + // we revert to just notify of the change.
>>> + // when the new value is not null, we also send the
>>> CHANGE on the source all the time, unless this was
>>> + // and added config bean.
>>> + if (!changed.contains(eventSource)) {
>>> + proxy =
>>> ConfigBeanProxy.class.cast(event.getSource());
>>> + changed.add(eventSource);
>>> + final NotProcessed nc =
>>> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
>>> + if ( nc != null ) {
>>> + unprocessed.add( new
>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>> }
>>> }
>>> } catch (Exception e) {
>>>
>>> Amy Roh wrote:
>>>
>>>> Attached the correct hk2 fix ;-)
>>>>
>>>> Amy Roh wrote:
>>>>
>>>>> Hi Jerome,
>>>>>
>>>>> I believe there's an oversight regarding hk2 revision 835 patch
>>>>> that was integreated for hk2 0.3.22.
>>>>>
>>>>> The attach diff fixes the oversight which ignored all CHANGE events
>>>>> when both event.getNewValue() and getOldValue() aren't NULL.
>>>>>
>>>>> Do we need another integration to include this fix?
>>>>>
>>>>> Thanks,
>>>>> Amy
>>>>>
>>>>> Amy Roh wrote:
>>>>>
>>>>>> I notice major regression on dynamic reconfig with the latest
>>>>>> build. Dynamic reconfig tests were passing last time web container
>>>>>> related changes were made on 9/10 and as late as of 9/12. I'm
>>>>>> trying to track down the cause. Pls. give me any headsup if you
>>>>>> are aware of any config changes that might have affected dynamic
>>>>>> reconfig.
>>>>>>
>>>>>> Thanks,
>>>>>> Amy
>>>>>
>>>>>
>>>>>
>>>>> Subject:
>>>>> svn commit: r835 -
>>>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>>> From:
>>>>> dochez_at_dev.java.net
>>>>> Date:
>>>>> Mon, 15 Sep 2008 05:21:19 +0000
>>>>> To:
>>>>> commits_at_hk2.dev.java.net
>>>>>
>>>>> To:
>>>>> commits_at_hk2.dev.java.net
>>>>>
>>>>>
>>>>> Author: dochez
>>>>> Date: 2008-09-15 05:21:17+0000
>>>>> New Revision: 835
>>>>>
>>>>> Modified:
>>>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>>>
>>>>> Log:
>>>>> fixed IT 6001 : NPE when old value is null in sortAndDispatch
>>>>>
>>>>>
>>>>>
>>>>> Modified:
>>>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>>> Url:
>>>>> https://hk2.dev.java.net/source/browse/hk2/trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java?view=diff&rev=835&p1=trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java&p2=trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java&r1=834&r2=835
>>>>>
>>>>> ==============================================================================
>>>>>
>>>>> ---
>>>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>>> (original)
>>>>> +++
>>>>> trunk/hk2/config/src/java/org/jvnet/hk2/config/ConfigSupport.java
>>>>> 2008-09-15 05:21:17+0000
>>>>> @@ -87,7 +87,7 @@
>>>>> public class ConfigSupport {
>>>>> /**
>>>>> - * Execute§ some logic on one config bean of type T protected
>>>>> by a transaction
>>>>> + * Execute� some logic on one config bean of type T
>>>>> protected by a transaction
>>>>> *
>>>>> * @param code code to execute
>>>>> * @param param config object participating in the transaction
>>>>> @@ -293,14 +293,23 @@
>>>>> continue;
>>>>> }
>>>>> if (event.getNewValue()==null) {
>>>>> - final ConfigBeanProxy proxy =
>>>>> ConfigBeanProxy.class.cast(event.getOldValue());
>>>>> - final NotProcessed nc =
>>>>> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
>>>>> - if ( nc != null ) {
>>>>> - unprocessed.add( new
>>>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>>>> + ConfigBeanProxy proxy = null;
>>>>> + try {
>>>>> + // getOldValue() can be null, we will
>>>>> notify a CHANGE event
>>>>> + proxy =
>>>>> ConfigBeanProxy.class.cast(event.getOldValue());
>>>>> + } catch (ClassCastException e) {
>>>>> + // this is ok, the old value was probably
>>>>> a string or something like this...
>>>>> + // we will notify the event.getSource()
>>>>> that it changed.
>>>>> + }
>>>>> + if (proxy!=null) {
>>>>> + final NotProcessed nc =
>>>>> target.changed(Changed.TYPE.REMOVE, proxyType(proxy), proxy );
>>>>> + if ( nc != null ) {
>>>>> + unprocessed.add( new
>>>>> UnprocessedChangeEvent(event, nc.getReason() ) );
>>>>> + }
>>>>> + continue;
>>>>> }
>>>>> - } else {
>>>>> if (!changed.contains(eventSource)) {
>>>>> - final ConfigBeanProxy proxy =
>>>>> ConfigBeanProxy.class.cast(event.getSource());
>>>>> + proxy =
>>>>> ConfigBeanProxy.class.cast(event.getSource());
>>>>> changed.add(eventSource);
>>>>> final NotProcessed nc =
>>>>> target.changed(Changed.TYPE.CHANGE, proxyType(proxy), proxy);
>>>>> if ( nc != null ) {
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: commits-unsubscribe_at_hk2.dev.java.net
>>>>> For additional commands, e-mail: commits-help_at_hk2.dev.java.net
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>>>
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>