You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@excalibur.apache.org by le...@apache.org on 2005/09/08 05:59:18 UTC
svn commit: r279498 - in
/excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl:
DefaultInstrumentManagerImpl.java InstrumentProxy.java
InstrumentableProxy.java
Author: leif
Date: Wed Sep 7 20:59:13 2005
New Revision: 279498
URL: http://svn.apache.org/viewcvs?rev=279498&view=rev
Log:
Fixed some potential NPEs caused by synchronization problems. They were noticed while looking at the code and not from an actual error. This problem has been in the code since the earliest versions.
It was possible (but unlikely) that an error could be thrown if instruments were being looked up while other threads were registering new ones at the same location in the tree. There was no danger if the contents of the instrument tree was not being modified, or if the modifications were not taking place while the tree was being browsed.
Modified:
excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/DefaultInstrumentManagerImpl.java
excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentProxy.java
excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentableProxy.java
Modified: excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/DefaultInstrumentManagerImpl.java
URL: http://svn.apache.org/viewcvs/excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/DefaultInstrumentManagerImpl.java?rev=279498&r1=279497&r2=279498&view=diff
==============================================================================
--- excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/DefaultInstrumentManagerImpl.java (original)
+++ excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/DefaultInstrumentManagerImpl.java Wed Sep 7 20:59:13 2005
@@ -1770,13 +1770,14 @@
{
synchronized( m_semaphore )
{
- m_instrumentableProxyArray = new InstrumentableProxy[ m_instrumentableProxies.size() ];
- m_instrumentableProxies.values().toArray( m_instrumentableProxyArray );
+ InstrumentableProxy[] instrumentableProxyArray =
+ new InstrumentableProxy[ m_instrumentableProxies.size() ];
+ m_instrumentableProxies.values().toArray( instrumentableProxyArray );
// Sort the array. This is not a performance problem because this
// method is rarely called and doing it here saves cycles in the
// client.
- Arrays.sort( m_instrumentableProxyArray, new Comparator()
+ Arrays.sort( instrumentableProxyArray, new Comparator()
{
public int compare( Object o1, Object o2 )
{
@@ -1790,7 +1791,11 @@
}
} );
- return m_instrumentableProxyArray;
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_instrumentableProxyArray = instrumentableProxyArray;
+
+ return instrumentableProxyArray;
}
}
@@ -1804,19 +1809,27 @@
{
synchronized( m_semaphore )
{
- if( m_instrumentableProxyArray == null )
+ // Get the proxy array. This is done in synchronization so it is not possible that it
+ // will be reset before we obtain the descriptor array. They are both set to null
+ // at the same time when there is a change.
+ InstrumentableProxy[] instrumentableProxyArray = m_instrumentableProxyArray;
+ if ( instrumentableProxyArray == null )
{
- updateInstrumentableProxyArray();
+ instrumentableProxyArray = updateInstrumentableProxyArray();
}
-
- m_instrumentableDescriptorArray =
- new InstrumentableDescriptor[ m_instrumentableProxyArray.length ];
- for( int i = 0; i < m_instrumentableProxyArray.length; i++ )
+
+ InstrumentableDescriptor[] instrumentableDescriptorArray =
+ new InstrumentableDescriptor[ instrumentableProxyArray.length ];
+ for( int i = 0; i < instrumentableProxyArray.length; i++ )
{
- m_instrumentableDescriptorArray[ i ] = m_instrumentableProxyArray[ i ].getDescriptor();
+ instrumentableDescriptorArray[ i ] = instrumentableProxyArray[ i ].getDescriptor();
}
-
- return m_instrumentableDescriptorArray;
+
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_instrumentableDescriptorArray = instrumentableDescriptorArray;
+
+ return instrumentableDescriptorArray;
}
}
Modified: excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentProxy.java
URL: http://svn.apache.org/viewcvs/excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentProxy.java?rev=279498&r1=279497&r2=279498&view=diff
==============================================================================
--- excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentProxy.java (original)
+++ excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentProxy.java Wed Sep 7 20:59:13 2005
@@ -943,13 +943,13 @@
{
synchronized(this)
{
- m_sampleArray = new InstrumentSample[ m_samples.size() ];
- m_samples.values().toArray( m_sampleArray );
+ InstrumentSample[] sampleArray = new InstrumentSample[ m_samples.size() ];
+ m_samples.values().toArray( sampleArray );
// Sort the array. This is not a performance problem because this
// method is rarely called and doing it here saves cycles in the
// client.
- Arrays.sort( m_sampleArray, new Comparator()
+ Arrays.sort( sampleArray, new Comparator()
{
public int compare( Object o1, Object o2 )
{
@@ -963,7 +963,12 @@
}
} );
- return m_sampleArray;
+
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_sampleArray = sampleArray;
+
+ return sampleArray;
}
}
@@ -977,19 +982,27 @@
{
synchronized(this)
{
- if ( m_sampleArray == null )
+ // Get the proxy array. This is done in synchronization so it is not possible that it
+ // will be reset before we obtain the descriptor array. They are both set to null
+ // at the same time when there is a change.
+ InstrumentSample[] sampleArray = m_sampleArray;
+ if ( sampleArray == null )
{
- updateInstrumentSampleArray();
+ sampleArray = updateInstrumentSampleArray();
}
- m_sampleDescriptorArray =
+ InstrumentSampleDescriptor[] sampleDescriptorArray =
new InstrumentSampleDescriptor[ m_sampleArray.length ];
- for ( int i = 0; i < m_sampleArray.length; i++ )
+ for ( int i = 0; i < sampleArray.length; i++ )
{
- m_sampleDescriptorArray[i] = m_sampleArray[i].getDescriptor();
+ sampleDescriptorArray[i] = sampleArray[i].getDescriptor();
}
- return m_sampleDescriptorArray;
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_sampleDescriptorArray = sampleDescriptorArray;
+
+ return sampleDescriptorArray;
}
}
Modified: excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentableProxy.java
URL: http://svn.apache.org/viewcvs/excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentableProxy.java?rev=279498&r1=279497&r2=279498&view=diff
==============================================================================
--- excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentableProxy.java (original)
+++ excalibur/trunk/containerkit/instrument/mgr-impl/src/java/org/apache/excalibur/instrument/manager/impl/InstrumentableProxy.java Wed Sep 7 20:59:13 2005
@@ -463,14 +463,14 @@
{
synchronized( this )
{
- m_childInstrumentableProxyArray =
+ InstrumentableProxy[] childInstrumentableProxyArray =
new InstrumentableProxy[ m_childInstrumentableProxies.size() ];
- m_childInstrumentableProxies.values().toArray( m_childInstrumentableProxyArray );
+ m_childInstrumentableProxies.values().toArray( childInstrumentableProxyArray );
// Sort the array. This is not a performance problem because this
// method is rarely called and doing it here saves cycles in the
// client.
- Arrays.sort( m_childInstrumentableProxyArray, new Comparator()
+ Arrays.sort( childInstrumentableProxyArray, new Comparator()
{
public int compare( Object o1, Object o2 )
{
@@ -484,7 +484,11 @@
}
} );
- return m_childInstrumentableProxyArray;
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_childInstrumentableProxyArray = childInstrumentableProxyArray;
+
+ return childInstrumentableProxyArray;
}
}
@@ -498,20 +502,28 @@
{
synchronized( this )
{
- if( m_childInstrumentableProxyArray == null )
+ // Get the proxy array. This is done in synchronization so it is not possible that it
+ // will be reset before we obtain the descriptor array. They are both set to null
+ // at the same time when there is a change.
+ InstrumentableProxy[] childInstrumentableProxyArray = m_childInstrumentableProxyArray;
+ if( childInstrumentableProxyArray == null )
{
- updateChildInstrumentableProxyArray();
+ childInstrumentableProxyArray = updateChildInstrumentableProxyArray();
}
- m_childInstrumentableDescriptorArray =
- new InstrumentableDescriptor[ m_childInstrumentableProxyArray.length ];
- for( int i = 0; i < m_childInstrumentableProxyArray.length; i++ )
+ InstrumentableDescriptor[] childInstrumentableDescriptorArray =
+ new InstrumentableDescriptor[ childInstrumentableProxyArray.length ];
+ for( int i = 0; i < childInstrumentableProxyArray.length; i++ )
{
- m_childInstrumentableDescriptorArray[ i ] =
- m_childInstrumentableProxyArray[ i ].getDescriptor();
+ childInstrumentableDescriptorArray[ i ] =
+ childInstrumentableProxyArray[ i ].getDescriptor();
}
- return m_childInstrumentableDescriptorArray;
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_childInstrumentableDescriptorArray = childInstrumentableDescriptorArray;
+
+ return childInstrumentableDescriptorArray;
}
}
@@ -671,13 +683,14 @@
{
synchronized( this )
{
- m_instrumentProxyArray = new InstrumentProxy[ m_instrumentProxies.size() ];
- m_instrumentProxies.values().toArray( m_instrumentProxyArray );
+ InstrumentProxy[] instrumentProxyArray =
+ new InstrumentProxy[ m_instrumentProxies.size() ];
+ m_instrumentProxies.values().toArray( instrumentProxyArray );
// Sort the array. This is not a performance problem because this
// method is rarely called and doing it here saves cycles in the
// client.
- Arrays.sort( m_instrumentProxyArray, new Comparator()
+ Arrays.sort( instrumentProxyArray, new Comparator()
{
public int compare( Object o1, Object o2 )
{
@@ -691,7 +704,11 @@
}
} );
- return m_instrumentProxyArray;
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_instrumentProxyArray = instrumentProxyArray;
+
+ return instrumentProxyArray;
}
}
@@ -705,19 +722,27 @@
{
synchronized( this )
{
- if( m_instrumentProxyArray == null )
+ // Get the proxy array. This is done in synchronization so it is not possible that it
+ // will be reset before we obtain the descriptor array. They are both set to null
+ // at the same time when there is a change.
+ InstrumentProxy[] instrumentProxyArray = m_instrumentProxyArray;
+ if( instrumentProxyArray == null )
{
- updateInstrumentProxyArray();
+ instrumentProxyArray = updateInstrumentProxyArray();
}
-
- m_instrumentDescriptorArray =
- new InstrumentDescriptor[ m_instrumentProxyArray.length ];
- for( int i = 0; i < m_instrumentProxyArray.length; i++ )
+
+ InstrumentDescriptor[] instrumentDescriptorArray =
+ new InstrumentDescriptor[ instrumentProxyArray.length ];
+ for( int i = 0; i < instrumentProxyArray.length; i++ )
{
- m_instrumentDescriptorArray[ i ] = m_instrumentProxyArray[ i ].getDescriptor();
+ instrumentDescriptorArray[ i ] = instrumentProxyArray[ i ].getDescriptor();
}
-
- return m_instrumentDescriptorArray;
+
+ // Once we are done modifying this array, set it to the variable accessable outside
+ // of synchronization.
+ m_instrumentDescriptorArray = instrumentDescriptorArray;
+
+ return instrumentDescriptorArray;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: scm-unsubscribe@excalibur.apache.org
For additional commands, e-mail: scm-help@excalibur.apache.org