SF1603631:
concurrent modification of ROT (adding or removing thread tables) causes VM crash. The simple fix is to sychronize all accessors to the ROT. Performance impact has not been analyzed
This commit is contained in:
@@ -45,7 +45,7 @@ public abstract class ROT {
|
|||||||
* Manual garbage collection was the only option pre 1.9
|
* Manual garbage collection was the only option pre 1.9
|
||||||
*/
|
*/
|
||||||
protected static final boolean USE_AUTOMATIC_GARBAGE_COLLECTION =
|
protected static final boolean USE_AUTOMATIC_GARBAGE_COLLECTION =
|
||||||
"true".equalsIgnoreCase(System.getProperty("com.jacob.autogc"));
|
"true".equalsIgnoreCase( System.getProperty( "com.jacob.autogc" ) );
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A hash table where each element is another
|
* A hash table where each element is another
|
||||||
@@ -59,38 +59,38 @@ public abstract class ROT {
|
|||||||
* adds a new thread storage area to rot
|
* adds a new thread storage area to rot
|
||||||
* @return Map corresponding to the thread that this call was made in
|
* @return Map corresponding to the thread that this call was made in
|
||||||
*/
|
*/
|
||||||
protected static Map addThread() {
|
protected synchronized static Map addThread() {
|
||||||
|
// should use the id here instead of the name because the name can be changed
|
||||||
String t_name = Thread.currentThread().getName();
|
String t_name = Thread.currentThread().getName();
|
||||||
if (rot.containsKey(t_name)){
|
if ( rot.containsKey( t_name ) ) {
|
||||||
// nothing to do
|
// nothing to do
|
||||||
} else {
|
} else {
|
||||||
Map tab = null;
|
Map tab = null;
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
JacobObject.debug("ROT: Automatic GC flag == "+ USE_AUTOMATIC_GARBAGE_COLLECTION
|
JacobObject.debug( "ROT: Automatic GC flag == " + USE_AUTOMATIC_GARBAGE_COLLECTION );
|
||||||
);}
|
}
|
||||||
if (!USE_AUTOMATIC_GARBAGE_COLLECTION){
|
if ( !USE_AUTOMATIC_GARBAGE_COLLECTION ) {
|
||||||
tab = new HashMap();
|
tab = new HashMap();
|
||||||
} else {
|
} else {
|
||||||
tab = new WeakHashMap();
|
tab = new WeakHashMap();
|
||||||
}
|
}
|
||||||
rot.put(t_name,tab);
|
rot.put( t_name, tab );
|
||||||
}
|
}
|
||||||
return getThreadObjects(false);
|
return getThreadObjects( false );
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* returns the pool for this thread if it exists. can create a new
|
* returns the pool for this thread if it exists. can create a new
|
||||||
* one if you wish by passing in TRUE
|
* one if you wish by passing in TRUE
|
||||||
* @param createIfDoesNotExist
|
* @param createIfDoesNotExist
|
||||||
* @return Map the collection that holds the objects created in the current thread
|
* @return Map the collection that holds the objects created in the current thread
|
||||||
*/
|
*/
|
||||||
protected static Map getThreadObjects(boolean createIfDoesNotExist) {
|
protected synchronized static Map getThreadObjects( boolean createIfDoesNotExist ) {
|
||||||
String t_name = Thread.currentThread().getName();
|
String t_name = Thread.currentThread().getName();
|
||||||
if (!rot.containsKey(t_name) && createIfDoesNotExist){
|
if ( !rot.containsKey( t_name ) && createIfDoesNotExist ) {
|
||||||
addThread();
|
addThread();
|
||||||
}
|
}
|
||||||
return (Map)rot.get(t_name);
|
return (Map) rot.get( t_name );
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -102,37 +102,37 @@ public abstract class ROT {
|
|||||||
* This is called by COMThread in the tear down and provides a
|
* This is called by COMThread in the tear down and provides a
|
||||||
* synchronous way of releasing memory
|
* synchronous way of releasing memory
|
||||||
*/
|
*/
|
||||||
protected static void clearObjects() {
|
protected synchronized static void clearObjects() {
|
||||||
Map tab = getThreadObjects(false);
|
Map tab = getThreadObjects( false );
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
JacobObject.debug("ROT: "+rot.keySet().size()+" thread tables exist");
|
JacobObject.debug( "ROT: " + rot.keySet().size() + " thread tables exist" );
|
||||||
}
|
}
|
||||||
if (tab != null){
|
if ( tab != null ) {
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
JacobObject.debug("ROT: "+tab.keySet().size()+ " objects to clear in this thread " );
|
JacobObject.debug( "ROT: " + tab.keySet().size() + " objects to clear in this thread " );
|
||||||
}
|
}
|
||||||
// walk the values
|
// walk the values
|
||||||
Iterator it;
|
Iterator it;
|
||||||
if (USE_AUTOMATIC_GARBAGE_COLLECTION){
|
if ( USE_AUTOMATIC_GARBAGE_COLLECTION ) {
|
||||||
it = tab.keySet().iterator();
|
it = tab.keySet().iterator();
|
||||||
} else {
|
} else {
|
||||||
it = tab.values().iterator();
|
it = tab.values().iterator();
|
||||||
}
|
}
|
||||||
while (it.hasNext()) {
|
while ( it.hasNext() ) {
|
||||||
JacobObject o = (JacobObject) it.next();
|
JacobObject o = (JacobObject) it.next();
|
||||||
if (o != null
|
if ( o != null
|
||||||
// can't use this cause creates a Variant if calling SafeAray
|
// can't use this cause creates a Variant if calling SafeAray
|
||||||
// and we get an exceptin modifying the collection while iterating
|
// and we get an exceptin modifying the collection while iterating
|
||||||
// && o.toString() != null
|
// && o.toString() != null
|
||||||
){
|
) {
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
if (o instanceof SafeArray){
|
if ( o instanceof SafeArray ) {
|
||||||
// SafeArray create more objects when calling toString()
|
// SafeArray create more objects when calling toString()
|
||||||
// which causes a concurrent modification exception in HashMap
|
// which causes a concurrent modification exception in HashMap
|
||||||
JacobObject.debug("ROT: removing "+o.getClass().getName());
|
JacobObject.debug( "ROT: removing " + o.getClass().getName() );
|
||||||
} else {
|
} else {
|
||||||
// Variant toString() is probably always bad in here
|
// Variant toString() is probably always bad in here
|
||||||
JacobObject.debug("ROT: removing "+o.hashCode()+"->"+o.getClass().getName());
|
JacobObject.debug( "ROT: removing " + o.hashCode() + "->" + o.getClass().getName() );
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
o.safeRelease();
|
o.safeRelease();
|
||||||
@@ -142,12 +142,14 @@ public abstract class ROT {
|
|||||||
// empty the collection
|
// empty the collection
|
||||||
tab.clear();
|
tab.clear();
|
||||||
// remove the collection from rot
|
// remove the collection from rot
|
||||||
rot.remove(Thread.currentThread().getName());
|
rot.remove( Thread.currentThread().getName() );
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
JacobObject.debug("ROT: thread table cleared and removed");
|
JacobObject.debug( "ROT: thread table cleared and removed" );
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if (JacobObject.isDebugEnabled()){ JacobObject.debug("ROT: nothing to clear!");}
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
|
JacobObject.debug( "ROT: nothing to clear!" );
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -157,11 +159,11 @@ public abstract class ROT {
|
|||||||
* @param o JacobObject we need key for
|
* @param o JacobObject we need key for
|
||||||
* @return some key compatabile with hashmaps
|
* @return some key compatabile with hashmaps
|
||||||
*/
|
*/
|
||||||
protected static Object getMapKey(Map targetMap, JacobObject o){
|
protected static Object getMapKey( Map targetMap, JacobObject o ) {
|
||||||
if (targetMap instanceof WeakHashMap){
|
if ( targetMap instanceof WeakHashMap ) {
|
||||||
return o;
|
return o;
|
||||||
} else {
|
} else {
|
||||||
return new Integer(o.hashCode());
|
return new Integer( o.hashCode() );
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -171,8 +173,8 @@ public abstract class ROT {
|
|||||||
* @param o JacobObject we need key for
|
* @param o JacobObject we need key for
|
||||||
* @return some compatabile with hashmaps (null for weak has map)
|
* @return some compatabile with hashmaps (null for weak has map)
|
||||||
*/
|
*/
|
||||||
protected static Object getMapValue(Map targetMap, JacobObject o){
|
protected static Object getMapValue( Map targetMap, JacobObject o ) {
|
||||||
if (targetMap instanceof WeakHashMap){
|
if ( targetMap instanceof WeakHashMap ) {
|
||||||
return null;
|
return null;
|
||||||
} else {
|
} else {
|
||||||
return o;
|
return o;
|
||||||
@@ -187,11 +189,11 @@ public abstract class ROT {
|
|||||||
* This will remove an object from the ROT
|
* This will remove an object from the ROT
|
||||||
* @param o
|
* @param o
|
||||||
*/
|
*/
|
||||||
protected static void removeObject(JacobObject o) {
|
protected synchronized static void removeObject( JacobObject o ) {
|
||||||
String t_name = Thread.currentThread().getName();
|
String t_name = Thread.currentThread().getName();
|
||||||
Map tab = (Map) rot.get(t_name);
|
Map tab = (Map) rot.get( t_name );
|
||||||
if (tab != null) {
|
if ( tab != null ) {
|
||||||
tab.remove(getMapKey(tab,o));
|
tab.remove( getMapKey( tab, o ) );
|
||||||
}
|
}
|
||||||
o.safeRelease();
|
o.safeRelease();
|
||||||
}
|
}
|
||||||
@@ -200,20 +202,21 @@ public abstract class ROT {
|
|||||||
* adds an object to the HashMap for the current thread
|
* adds an object to the HashMap for the current thread
|
||||||
* @param o
|
* @param o
|
||||||
*/
|
*/
|
||||||
protected static void addObject(JacobObject o) {
|
protected synchronized static void addObject( JacobObject o ) {
|
||||||
Map tab = getThreadObjects(false);
|
Map tab = getThreadObjects( false );
|
||||||
if (tab == null) {
|
if ( tab == null ) {
|
||||||
// this thread has not been initialized as a COM thread
|
// this thread has not been initialized as a COM thread
|
||||||
// so make it part of MTA for backwards compatibility
|
// so make it part of MTA for backwards compatibility
|
||||||
ComThread.InitMTA(false);
|
ComThread.InitMTA( false );
|
||||||
tab = getThreadObjects(true);
|
tab = getThreadObjects( true );
|
||||||
}
|
}
|
||||||
if (JacobObject.isDebugEnabled()){
|
if ( JacobObject.isDebugEnabled() ) {
|
||||||
JacobObject.debug(
|
JacobObject.debug(
|
||||||
"ROT: adding "+o+"->"+o.getClass().getName() +
|
"ROT: adding " + o + "->" + o.getClass().getName() +
|
||||||
" table size prior to addition:" +tab.size());}
|
" table size prior to addition:" + tab.size() );
|
||||||
if (tab != null) {
|
}
|
||||||
tab.put(getMapKey(tab,o),getMapValue(tab,o));
|
if ( tab != null ) {
|
||||||
|
tab.put( getMapKey( tab, o ), getMapValue( tab, o ) );
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -226,3 +229,4 @@ public abstract class ROT {
|
|||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user