dev@glassfish.java.net

Re: regression on dynamic reconfig

From: Jerome Dochez <Jerome.Dochez_at_Sun.COM>
Date: Tue, 16 Sep 2008 21:17:44 -0700

indeed I can see there is a problem :(
however, I think your fix might still not be complete...

can you try mine :)

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