dev@glassfish.java.net

Re: regression on dynamic reconfig

From: Jerome Dochez <Jerome.Dochez_at_Sun.COM>
Date: Tue, 16 Sep 2008 22:43:02 -0700

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 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