Skip to content

Commit a9fa229

Browse files
committed
Preserve order in ThreadGroup#list.
This is a squashed commit of the following changes for #2221. Thanks to @godfat for the work on this one. commit 4bc9e8e Author: Lin Jen-Shin <godfat@godfat.org> Date: Fri Dec 19 04:13:20 2014 +0800 WeakIdentityLinkedHashMap: Properly set head. commit fe56e69 Author: Lin Jen-Shin <godfat@godfat.org> Date: Fri Dec 19 02:10:43 2014 +0800 Use WeakIdentityLinkedHashSet for RubyThreadGroup: We extend WeakIdentityHashMap for WeakIdentityLinkedHashMap, and use it for WeakIdentityLinkedHashSet. Sorry that this was a naive implementation and I didn't write any tests for this. Originally I want to do something like java.util.LinkedHashMap, but it seems WeakIdentityHashMap didn't implement every thing we need to do the same. Therefore I decided to just write what we need for RubyThreadGroup. We could complete this in the future. Note that this should also fix a bug where WeakIdentityHashMap#valueRemoved is not called properly. commit e5bcad9 Author: Lin Jen-Shin <godfat@godfat.org> Date: Tue Nov 25 00:43:53 2014 +0800 Use a ReferenceQueue to sweep dead references commit 475746f Author: Lin Jen-Shin <godfat@godfat.org> Date: Fri Nov 21 08:06:39 2014 +0800 Preserve ThreadGroup#list order with ArrayList Because MRI uses a linked list which preserves the order of the threads. https://github.com/ruby/ruby/blob/2a754a733045da9965e88d1f31e650ea6b3f2b6c/vm_core.h#L973-L978 This is useful to assume the first thread in the list is the root of a group, and saving values in the thread could be shared amongst the thread group. If we ever have thread group local variable, this is not really needed.
1 parent 4f21b1e commit a9fa229

File tree

3 files changed

+121
-13
lines changed

3 files changed

+121
-13
lines changed

core/src/main/java/org/jruby/RubyThreadGroup.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,17 @@
2929
***** END LICENSE BLOCK *****/
3030
package org.jruby;
3131

32+
import java.lang.ref.WeakReference;
3233
import java.util.Collections;
3334
import java.util.Set;
3435
import org.jruby.anno.JRubyMethod;
3536
import org.jruby.anno.JRubyClass;
3637

38+
import org.jruby.util.WeakIdentityLinkedHashSet;
3739
import org.jruby.runtime.Block;
3840
import org.jruby.runtime.ClassIndex;
3941
import org.jruby.runtime.ObjectAllocator;
4042
import org.jruby.runtime.builtin.IRubyObject;
41-
import org.jruby.util.collections.WeakHashSet;
4243

4344
/**
4445
* Implementation of Ruby's <code>ThreadGroup</code> class. This is currently
@@ -49,7 +50,7 @@
4950
*/
5051
@JRubyClass(name="ThreadGroup")
5152
public class RubyThreadGroup extends RubyObject {
52-
private final Set<RubyThread> rubyThreadList = Collections.synchronizedSet(new WeakHashSet<RubyThread>());
53+
private final Set rubyThreadList = Collections.synchronizedSet(new WeakIdentityLinkedHashSet());
5354
private boolean enclosed = false;
5455

5556
public static RubyClass createThreadGroupClass(Ruby runtime) {
@@ -145,11 +146,10 @@ public IRubyObject enclosed_p(Block block) {
145146
@JRubyMethod
146147
public IRubyObject list(Block block) {
147148
RubyArray ary = RubyArray.newArray(getRuntime());
148-
synchronized (ary) {
149-
for (RubyThread thread : rubyThreadList) {
150-
if (thread != null) {
151-
ary.append(thread);
152-
}
149+
synchronized (ary) {
150+
for (Object obj : rubyThreadList) {
151+
RubyThread thread = (RubyThread) obj;
152+
ary.add(thread);
153153
}
154154
return ary;
155155
}
@@ -158,5 +158,4 @@ public IRubyObject list(Block block) {
158158
private RubyThreadGroup(Ruby runtime, RubyClass type) {
159159
super(runtime, type);
160160
}
161-
162161
}

core/src/main/java/org/jruby/util/WeakIdentityHashMap.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ private void clear(int size) {
151151
table = new Entry[range];
152152
}
153153

154-
private void expunge() {
154+
protected void expunge() {
155155
Entry e;
156156
while ((e = (Entry) queue.poll()) != null) {
157157
removeEntry(e);
@@ -219,13 +219,17 @@ private Object put(int hash, Object masked_key, Object value) {
219219
idx = index(hash);
220220
}
221221

222-
table[idx] = new Entry(hash, masked_key, value, table[idx], queue);
222+
table[idx] = newEntry(hash, masked_key, value, table[idx], queue);
223223

224224
size += 1;
225225

226226
return null;
227227
}
228228

229+
protected Entry newEntry(int hash, Object masked_key, Object value, Entry next, ReferenceQueue queue) {
230+
return new Entry(hash, masked_key, value, next, queue);
231+
}
232+
229233
public Object remove(Object key) {
230234
key = maskKey(key);
231235
int hash = keyHash(key);
@@ -242,6 +246,7 @@ public Object remove(int hash, Object key) {
242246
if (entry.sameKey(hash, key)) {
243247
table[idx] = entry.next;
244248
size -= 1;
249+
entryRemoved(entry);
245250
return entry.getValue();
246251

247252
} else {
@@ -251,6 +256,7 @@ public Object remove(int hash, Object key) {
251256
if (ahead.sameKey(hash, key)) {
252257
entry.next = ahead.next;
253258
size -= 1;
259+
entryRemoved(ahead);
254260
return ahead.getValue();
255261
}
256262

@@ -264,7 +270,7 @@ public Object remove(int hash, Object key) {
264270
return null;
265271
}
266272

267-
private void removeEntry(Entry ent) {
273+
protected void removeEntry(Entry ent) {
268274
int idx = index(ent.key_hash);
269275

270276
Entry entry = table[idx];
@@ -273,6 +279,7 @@ private void removeEntry(Entry ent) {
273279
if (entry == ent) {
274280
table[idx] = entry.next;
275281
size -= 1;
282+
entryRemoved(entry);
276283
return;
277284

278285
} else {
@@ -282,6 +289,7 @@ private void removeEntry(Entry ent) {
282289
if (ahead == ent) {
283290
entry.next = ahead.next;
284291
size -= 1;
292+
entryRemoved(ahead);
285293
return;
286294
}
287295

@@ -290,8 +298,11 @@ private void removeEntry(Entry ent) {
290298
}
291299
}
292300
}
293-
294-
valueRemoved(ent.value);
301+
}
302+
303+
// can be overridden to be informed when entries are removed
304+
protected void entryRemoved(Entry entry) {
305+
valueRemoved(entry.value);
295306
}
296307

297308
// can be overridden to be informed when objects are removed
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
2+
package org.jruby.util;
3+
4+
import java.lang.ref.ReferenceQueue;
5+
import java.util.Iterator;
6+
import java.util.NoSuchElementException;
7+
8+
public class WeakIdentityLinkedHashMap extends WeakIdentityHashMap {
9+
public WeakIdentityLinkedHashMap() {
10+
super();
11+
}
12+
13+
public WeakIdentityLinkedHashMap(int size) {
14+
super(size);
15+
}
16+
17+
class Entry extends WeakIdentityHashMap.Entry {
18+
Entry before, after;
19+
20+
Entry(int hash, Object masked_key, Object value, WeakIdentityHashMap.Entry next, ReferenceQueue queue, Entry tail) {
21+
super(hash, masked_key, value, next, queue);
22+
before = tail;
23+
if (tail != null) {
24+
tail.after = this;
25+
}
26+
}
27+
}
28+
29+
// The head (eldest) of the doubly linked list.
30+
transient Entry head;
31+
32+
// The tail (youngest) of the doubly linked list.
33+
transient Entry tail;
34+
35+
@Override
36+
public void clear() {
37+
head = tail = null;
38+
super.clear();
39+
}
40+
41+
@Override
42+
protected WeakIdentityHashMap.Entry newEntry(int hash, Object masked_key, Object value, WeakIdentityHashMap.Entry next, ReferenceQueue queue) {
43+
Entry newTail = new Entry(hash, masked_key, value, next, queue, tail);
44+
if (head == null) {
45+
head = newTail;
46+
}
47+
tail = newTail;
48+
return newTail;
49+
}
50+
51+
@Override
52+
protected void entryRemoved(WeakIdentityHashMap.Entry entry) {
53+
Entry ent = (Entry) entry;
54+
if (ent.before == null) {
55+
head = ent.after;
56+
}
57+
else {
58+
ent.before.after = ent.after;
59+
}
60+
super.entryRemoved(entry);
61+
}
62+
63+
@Override
64+
protected Iterator entryIterator() {
65+
return new EntryIterator();
66+
}
67+
68+
final class EntryIterator implements Iterator {
69+
private Entry entry;
70+
71+
EntryIterator() {
72+
expunge();
73+
entry = head;
74+
}
75+
76+
public boolean hasNext() {
77+
return (entry != null);
78+
}
79+
80+
public Object next() {
81+
Object result = entry;
82+
83+
if (result == null) {
84+
throw new NoSuchElementException();
85+
} else {
86+
entry = entry.after;
87+
return result;
88+
}
89+
}
90+
91+
public void remove() {
92+
Entry removed = entry;
93+
expunge();
94+
entry = entry.after;
95+
WeakIdentityLinkedHashMap.this.removeEntry(removed);
96+
}
97+
}
98+
}

0 commit comments

Comments
 (0)