Skip to content

Commit f1ff7e2

Browse files
author
clay_shooter
committed
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
1 parent d9b7eb9 commit f1ff7e2

File tree

1 file changed

+184
-180
lines changed

1 file changed

+184
-180
lines changed

‎jacob/src/com/jacob/com/ROT.java

Lines changed: 184 additions & 180 deletions
Original file line numberDiff line numberDiff line change
@@ -41,188 +41,192 @@
4141
* Is [ 1116101 ] jacob-msg 0284 relevant???
4242
*/
4343
public abstract class ROT {
44-
/**
45-
* Manual garbage collection was the only option pre 1.9
46-
*/
47-
protected static final boolean USE_AUTOMATIC_GARBAGE_COLLECTION =
48-
"true".equalsIgnoreCase(System.getProperty("com.jacob.autogc"));
44+
/**
45+
* Manual garbage collection was the only option pre 1.9
46+
*/
47+
protected static final boolean USE_AUTOMATIC_GARBAGE_COLLECTION =
48+
"true".equalsIgnoreCase( System.getProperty( "com.jacob.autogc" ) );
4949

50-
/**
51-
* A hash table where each element is another
52-
* HashMap that represents a thread.
53-
* Each thread HashMap contains the com objects created
54-
* in that thread
55-
*/
56-
private static HashMap rot = new HashMap();
50+
/**
51+
* A hash table where each element is another
52+
* HashMap that represents a thread.
53+
* Each thread HashMap contains the com objects created
54+
* in that thread
55+
*/
56+
private static HashMap rot = new HashMap();
5757

58-
/**
59-
* adds a new thread storage area to rot
60-
* @return Map corresponding to the thread that this call was made in
61-
*/
62-
protected static Map addThread() {
63-
String t_name = Thread.currentThread().getName();
64-
if (rot.containsKey(t_name)){
65-
// nothing to do
66-
} else {
67-
Map tab = null;
68-
if (JacobObject.isDebugEnabled()){
69-
JacobObject.debug("ROT: Automatic GC flag == "+ USE_AUTOMATIC_GARBAGE_COLLECTION
70-
);}
71-
if (!USE_AUTOMATIC_GARBAGE_COLLECTION){
72-
tab = new HashMap();
73-
} else {
74-
tab = new WeakHashMap();
75-
}
76-
rot.put(t_name,tab);
77-
}
78-
return getThreadObjects(false);
79-
}
58+
/**
59+
* adds a new thread storage area to rot
60+
* @return Map corresponding to the thread that this call was made in
61+
*/
62+
protected synchronized static Map addThread() {
63+
// should use the id here instead of the name because the name can be changed
64+
String t_name = Thread.currentThread().getName();
65+
if ( rot.containsKey( t_name ) ) {
66+
// nothing to do
67+
} else {
68+
Map tab = null;
69+
if ( JacobObject.isDebugEnabled() ) {
70+
JacobObject.debug( "ROT: Automatic GC flag == " + USE_AUTOMATIC_GARBAGE_COLLECTION );
71+
}
72+
if ( !USE_AUTOMATIC_GARBAGE_COLLECTION ) {
73+
tab = new HashMap();
74+
} else {
75+
tab = new WeakHashMap();
76+
}
77+
rot.put( t_name, tab );
78+
}
79+
return getThreadObjects( false );
80+
}
8081

81-
82-
/**
83-
* returns the pool for this thread if it exists. can create a new
84-
* one if you wish by passing in TRUE
85-
* @param createIfDoesNotExist
86-
* @return Map the collection that holds the objects created in the current thread
87-
*/
88-
protected static Map getThreadObjects(boolean createIfDoesNotExist) {
89-
String t_name = Thread.currentThread().getName();
90-
if (!rot.containsKey(t_name) && createIfDoesNotExist){
91-
addThread();
92-
}
93-
return (Map)rot.get(t_name);
94-
}
95-
96-
/**
97-
* Iterates across all of the entries in the Hashmap in the rot
98-
* that corresponds to this thread.
99-
* This calls safeRelease() on each entry and then
100-
* clears the map when done and removes it from the rot.
101-
* All traces of this thread's objects will disapear.
102-
* This is called by COMThread in the tear down and provides a
103-
* synchronous way of releasing memory
104-
*/
105-
protected static void clearObjects() {
106-
Map tab = getThreadObjects(false);
107-
if (JacobObject.isDebugEnabled()){
108-
JacobObject.debug("ROT: "+rot.keySet().size()+" thread tables exist");
109-
}
110-
if (tab != null){
111-
if (JacobObject.isDebugEnabled()){
112-
JacobObject.debug("ROT: "+tab.keySet().size()+ " objects to clear in this thread " );
113-
}
114-
// walk the values
115-
Iterator it;
116-
if (USE_AUTOMATIC_GARBAGE_COLLECTION){
117-
it = tab.keySet().iterator();
118-
} else {
119-
it = tab.values().iterator();
120-
}
121-
while (it.hasNext()) {
122-
JacobObject o = (JacobObject) it.next();
123-
if (o != null
124-
// can't use this cause creates a Variant if calling SafeAray
125-
// and we get an exceptin modifying the collection while iterating
126-
// && o.toString() != null
127-
){
128-
if (JacobObject.isDebugEnabled()){
129-
if (o instanceof SafeArray){
130-
// SafeArray create more objects when calling toString()
131-
// which causes a concurrent modification exception in HashMap
132-
JacobObject.debug("ROT: removing "+o.getClass().getName());
133-
} else {
134-
// Variant toString() is probably always bad in here
135-
JacobObject.debug("ROT: removing "+o.hashCode()+"->"+o.getClass().getName());
136-
}
137-
}
138-
o.safeRelease();
139-
}
140-
// used to be an iterator.remove() but why bother when we're nuking them all anyway?
141-
}
142-
// empty the collection
143-
tab.clear();
144-
// remove the collection from rot
145-
rot.remove(Thread.currentThread().getName());
146-
if (JacobObject.isDebugEnabled()){
147-
JacobObject.debug("ROT: thread table cleared and removed");
148-
}
149-
} else {
150-
if (JacobObject.isDebugEnabled()){ JacobObject.debug("ROT: nothing to clear!");}
151-
}
152-
}
82+
/**
83+
* returns the pool for this thread if it exists. can create a new
84+
* one if you wish by passing in TRUE
85+
* @param createIfDoesNotExist
86+
* @return Map the collection that holds the objects created in the current thread
87+
*/
88+
protected synchronized static Map getThreadObjects( boolean createIfDoesNotExist ) {
89+
String t_name = Thread.currentThread().getName();
90+
if ( !rot.containsKey( t_name ) && createIfDoesNotExist ) {
91+
addThread();
92+
}
93+
return (Map) rot.get( t_name );
94+
}
15395

154-
/**
155-
* generates the key used to insert object into the thread's list of objects.
156-
* @param targetMap the map we need the key for. Used to make sure we create a compatabile key
157-
* @param o JacobObject we need key for
158-
* @return some key compatabile with hashmaps
159-
*/
160-
protected static Object getMapKey(Map targetMap, JacobObject o){
161-
if (targetMap instanceof WeakHashMap){
162-
return o;
163-
} else {
164-
return new Integer(o.hashCode());
165-
}
166-
}
167-
168-
/**
169-
* generates the object used to insert object into the thread's list of objects.
170-
* @param targetMap the map we need the key for. Used to make sure we create a compatabile key
171-
* @param o JacobObject we need key for
172-
* @return some compatabile with hashmaps (null for weak has map)
173-
*/
174-
protected static Object getMapValue(Map targetMap, JacobObject o){
175-
if (targetMap instanceof WeakHashMap){
176-
return null;
177-
} else {
178-
return o;
179-
}
180-
}
181-
182-
/**
183-
* @deprecated the java model leave the responsibility of clearing up objects
184-
* to the Garbage Collector. Our programming model should not require that the
185-
* user specifically remove object from the thread.
186-
*
187-
* This will remove an object from the ROT
188-
* @param o
189-
*/
190-
protected static void removeObject(JacobObject o) {
191-
String t_name = Thread.currentThread().getName();
192-
Map tab = (Map) rot.get(t_name);
193-
if (tab != null) {
194-
tab.remove(getMapKey(tab,o));
195-
}
196-
o.safeRelease();
197-
}
198-
199-
/**
200-
* adds an object to the HashMap for the current thread
201-
* @param o
202-
*/
203-
protected static void addObject(JacobObject o) {
204-
Map tab = getThreadObjects(false);
205-
if (tab == null) {
206-
// this thread has not been initialized as a COM thread
207-
// so make it part of MTA for backwards compatibility
208-
ComThread.InitMTA(false);
209-
tab = getThreadObjects(true);
210-
}
211-
if (JacobObject.isDebugEnabled()){
212-
JacobObject.debug(
213-
"ROT: adding "+o+"->"+o.getClass().getName() +
214-
" table size prior to addition:" +tab.size());}
215-
if (tab != null) {
216-
tab.put(getMapKey(tab,o),getMapValue(tab,o));
217-
}
218-
}
96+
/**
97+
* Iterates across all of the entries in the Hashmap in the rot
98+
* that corresponds to this thread.
99+
* This calls safeRelease() on each entry and then
100+
* clears the map when done and removes it from the rot.
101+
* All traces of this thread's objects will disapear.
102+
* This is called by COMThread in the tear down and provides a
103+
* synchronous way of releasing memory
104+
*/
105+
protected synchronized static void clearObjects() {
106+
Map tab = getThreadObjects( false );
107+
if ( JacobObject.isDebugEnabled() ) {
108+
JacobObject.debug( "ROT: " + rot.keySet().size() + " thread tables exist" );
109+
}
110+
if ( tab != null ) {
111+
if ( JacobObject.isDebugEnabled() ) {
112+
JacobObject.debug( "ROT: " + tab.keySet().size() + " objects to clear in this thread " );
113+
}
114+
// walk the values
115+
Iterator it;
116+
if ( USE_AUTOMATIC_GARBAGE_COLLECTION ) {
117+
it = tab.keySet().iterator();
118+
} else {
119+
it = tab.values().iterator();
120+
}
121+
while ( it.hasNext() ) {
122+
JacobObject o = (JacobObject) it.next();
123+
if ( o != null
124+
// can't use this cause creates a Variant if calling SafeAray
125+
// and we get an exceptin modifying the collection while iterating
126+
// && o.toString() != null
127+
) {
128+
if ( JacobObject.isDebugEnabled() ) {
129+
if ( o instanceof SafeArray ) {
130+
// SafeArray create more objects when calling toString()
131+
// which causes a concurrent modification exception in HashMap
132+
JacobObject.debug( "ROT: removing " + o.getClass().getName() );
133+
} else {
134+
// Variant toString() is probably always bad in here
135+
JacobObject.debug( "ROT: removing " + o.hashCode() + "->" + o.getClass().getName() );
136+
}
137+
}
138+
o.safeRelease();
139+
}
140+
// used to be an iterator.remove() but why bother when we're nuking them all anyway?
141+
}
142+
// empty the collection
143+
tab.clear();
144+
// remove the collection from rot
145+
rot.remove( Thread.currentThread().getName() );
146+
if ( JacobObject.isDebugEnabled() ) {
147+
JacobObject.debug( "ROT: thread table cleared and removed" );
148+
}
149+
} else {
150+
if ( JacobObject.isDebugEnabled() ) {
151+
JacobObject.debug( "ROT: nothing to clear!" );
152+
}
153+
}
154+
}
219155

220-
/**
221-
* ROT can't be a subclass of JacobObject because of the way ROT pools are managed
222-
* so we force a DLL load here by referncing JacobObject
223-
*/
224-
static {
225-
LibraryLoader.loadJacobLibrary();
226-
}
227-
228-
}
156+
/**
157+
* generates the key used to insert object into the thread's list of objects.
158+
* @param targetMap the map we need the key for. Used to make sure we create a compatabile key
159+
* @param o JacobObject we need key for
160+
* @return some key compatabile with hashmaps
161+
*/
162+
protected static Object getMapKey( Map targetMap, JacobObject o ) {
163+
if ( targetMap instanceof WeakHashMap ) {
164+
return o;
165+
} else {
166+
return new Integer( o.hashCode() );
167+
}
168+
}
169+
170+
/**
171+
* generates the object used to insert object into the thread's list of objects.
172+
* @param targetMap the map we need the key for. Used to make sure we create a compatabile key
173+
* @param o JacobObject we need key for
174+
* @return some compatabile with hashmaps (null for weak has map)
175+
*/
176+
protected static Object getMapValue( Map targetMap, JacobObject o ) {
177+
if ( targetMap instanceof WeakHashMap ) {
178+
return null;
179+
} else {
180+
return o;
181+
}
182+
}
183+
184+
/**
185+
* @deprecated the java model leave the responsibility of clearing up objects
186+
* to the Garbage Collector. Our programming model should not require that the
187+
* user specifically remove object from the thread.
188+
*
189+
* This will remove an object from the ROT
190+
* @param o
191+
*/
192+
protected synchronized static void removeObject( JacobObject o ) {
193+
String t_name = Thread.currentThread().getName();
194+
Map tab = (Map) rot.get( t_name );
195+
if ( tab != null ) {
196+
tab.remove( getMapKey( tab, o ) );
197+
}
198+
o.safeRelease();
199+
}
200+
201+
/**
202+
* adds an object to the HashMap for the current thread
203+
* @param o
204+
*/
205+
protected synchronized static void addObject( JacobObject o ) {
206+
Map tab = getThreadObjects( false );
207+
if ( tab == null ) {
208+
// this thread has not been initialized as a COM thread
209+
// so make it part of MTA for backwards compatibility
210+
ComThread.InitMTA( false );
211+
tab = getThreadObjects( true );
212+
}
213+
if ( JacobObject.isDebugEnabled() ) {
214+
JacobObject.debug(
215+
"ROT: adding " + o + "->" + o.getClass().getName() +
216+
" table size prior to addition:" + tab.size() );
217+
}
218+
if ( tab != null ) {
219+
tab.put( getMapKey( tab, o ), getMapValue( tab, o ) );
220+
}
221+
}
222+
223+
/**
224+
* ROT can't be a subclass of JacobObject because of the way ROT pools are managed
225+
* so we force a DLL load here by referncing JacobObject
226+
*/
227+
static {
228+
LibraryLoader.loadJacobLibrary();
229+
}
230+
231+
}
232+

0 commit comments

Comments
 (0)