Skip to content

Commit d86ca6f

Browse files
ponderosaTXponderosaTX
committed
Converted _listeners in EventNotifier to CopyOnWriteArrayList, simplifying readers/writers synchronization
On branch java8-alt Your branch is up to date with 'origin/java8-alt'. Changes to be committed: modified: drjava/src/edu/rice/cs/drjava/model/AbstractDJDocument.java modified: drjava/src/edu/rice/cs/drjava/model/BrowserHistoryManager.java modified: drjava/src/edu/rice/cs/drjava/model/ConcreteRegionManager.java modified: drjava/src/edu/rice/cs/drjava/model/EventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/GlobalEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/compiler/CompilerEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/debug/DebugEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/javadoc/JavadocEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/junit/JUnitEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/repl/InteractionsEventNotifier.java modified: drjava/src/edu/rice/cs/drjava/model/repl/newjvm/MainJVM.java modified: drjava/src/edu/rice/cs/drjava/ui/avail/GUIAvailabilityNotifier.java Untracked files: drjava/src/edu/rice/cs/drjava/model/EventNotifier.java.alt2
1 parent 93db0ca commit d86ca6f

File tree

12 files changed

+220
-561
lines changed

12 files changed

+220
-561
lines changed

‎drjava/src/edu/rice/cs/drjava/model/AbstractDJDocument.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,10 +1686,8 @@ private int _getWhiteSpace() throws BadLocationException {
16861686
* @throws BadLocationException if attempts to reference an invalid location
16871687
*/
16881688
private int _getWhiteSpacePrefix() throws BadLocationException {
1689-
1690-
// System.err.println("lockState = " + _lockState);
1691-
1692-
/* */ assert Utilities.TEST_MODE || EventQueue.isDispatchThread();
1689+
1690+
assert Utilities.TEST_MODE || EventQueue.isDispatchThread();
16931691

16941692
int lineStart = _getLineStartPos(_currentLocation);
16951693
if (lineStart < 0) lineStart = 0; // _currentLocation on first line

‎drjava/src/edu/rice/cs/drjava/model/BrowserHistoryManager.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,7 @@ public void addBrowserRegion(final BrowserDocumentRegion r, final GlobalEventNot
7878
// Notify listeners of this event
7979
Utilities.invokeLater(new Runnable() {
8080
public void run() {
81-
_lock.startRead();
82-
try { for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionAdded(r); } }
83-
finally { _lock.endRead(); }
81+
for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionAdded(r); }
8482
}
8583
});
8684

@@ -113,9 +111,7 @@ public void addBrowserRegionBefore(final BrowserDocumentRegion r, final GlobalEv
113111
// Notify listeners of this event
114112
Utilities.invokeLater(new Runnable() {
115113
public void run() {
116-
_lock.startRead();
117-
try { for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionAdded(r); } }
118-
finally { _lock.endRead(); }
114+
for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionAdded(r); }
119115
}
120116
});
121117

@@ -153,9 +149,7 @@ public void removeLast(final ArrayDeque<BrowserDocumentRegion> stack) {
153149
// Notify listeners of this event
154150
Utilities.invokeLater(new Runnable() {
155151
public void run() {
156-
_lock.startRead();
157-
try { for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionRemoved(r); } }
158-
finally { _lock.endRead(); }
152+
for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionRemoved(r); }
159153
}
160154
});
161155
}
@@ -172,9 +166,7 @@ public void remove(final BrowserDocumentRegion r) {
172166
// Notify listeners of this event
173167
Utilities.invokeLater(new Runnable() {
174168
public void run() {
175-
_lock.startRead();
176-
try { for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionRemoved(r); } }
177-
finally { _lock.endRead(); }
169+
for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionRemoved(r); }
178170
}
179171
});
180172
}
@@ -244,11 +236,8 @@ public void changeRegion(final BrowserDocumentRegion region, Lambda<BrowserDocum
244236
cmd.value(region);
245237
Utilities.invokeLater(new Runnable() { public void run() {
246238
// notify
247-
_lock.startRead();
248-
try {
249239
for (RegionManagerListener<BrowserDocumentRegion> l: _listeners) { l.regionChanged(region); }
250-
} finally { _lock.endRead(); }
251-
} });
240+
}});
252241
}
253242

254243
/** @param r1 the first region to compare

‎drjava/src/edu/rice/cs/drjava/model/ConcreteRegionManager.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,7 @@ public void addRegion(final R region) {
332332

333333
// only notify if the region was actually added
334334
if (! alreadyPresent) {
335-
// notify. invokeLater unnecessary if it only runs in the event thread
336-
_lock.startRead();
337-
try { for (RegionManagerListener<R> l: _listeners) { l.regionAdded(region); } }
338-
finally { _lock.endRead(); }
335+
for (RegionManagerListener<R> l: _listeners) { l.regionAdded(region); }
339336
}
340337
}
341338

@@ -366,9 +363,7 @@ public void removeRegions(Iterable<? extends R> regions) {
366363
}
367364

368365
private void _notifyRegionRemoved(final R region) {
369-
_lock.startRead();
370-
try { for (RegionManagerListener<R> l: _listeners) { l.regionRemoved(region); } }
371-
finally { _lock.endRead(); }
366+
for (RegionManagerListener<R> l: _listeners) { l.regionRemoved(region); }
372367
}
373368

374369
/** Remove the specified document from _documents and _regions (removing all
@@ -466,9 +461,7 @@ public void changeRegion(final R region, Lambda<R,Object> cmd) {
466461
* @param region the region that changed
467462
*/
468463
public void notifyChangedRegion(final R region) {
469-
_lock.startRead();
470-
try { for (RegionManagerListener<R> l: _listeners) { l.regionChanged(region); } }
471-
finally { _lock.endRead(); }
464+
for (RegionManagerListener<R> l: _listeners) { l.regionChanged(region); }
472465
}
473466

474467
/** Updates _lineStartPos, _lineEndPos of regions in the interval

‎drjava/src/edu/rice/cs/drjava/model/EventNotifier.java‎

Lines changed: 7 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
* END_COPYRIGHT_BLOCK*/
2929
package edu.rice.cs.drjava.model;
3030

31-
import java.util.LinkedList;
31+
import java.util.List;
32+
import java.util.concurrent.CopyOnWriteArrayList;
3233
import edu.rice.cs.util.ReaderWriterLock;
3334

3435
/** Base class for all component-specific EventNotifiers. This class provides common methods to
@@ -39,69 +40,24 @@ public abstract class EventNotifier<T> {
3940
/** All T Listeners that are listening to the model. Accesses to this collection are protected by the
4041
* ReaderWriterLock. The collection must be synchronized, since multiple readers could access it at once.
4142
*/
42-
protected final LinkedList<T> _listeners = new LinkedList<T>();
43+
protected final List<T> _listeners = new CopyOnWriteArrayList<T>();
4344

44-
/** Provides synchronization primitives for solving the readers/writers problem. In EventNotifier, adding and
45-
* removing listeners are considered write operations, and all notifications are considered read operations. Multiple
46-
* reads can occur simultaneously, but only one write can occur at a time, and no reads can occur during a write.
47-
*/
48-
protected final ReaderWriterLock _lock = new ReaderWriterLock();
45+
/* The listener framework is now implemented using CopyOnWriteArrayList, eliminating the readers/writers issue. */
4946

5047
/** Adds a listener to the notifier.
5148
* @param listener a listener that reacts on events
5249
*/
53-
public void addListener(T listener) {
54-
_lock.startWrite();
55-
try { _listeners.add(listener); }
56-
finally { _lock.endWrite(); }
57-
}
50+
public void addListener(T listener) { _listeners.add(listener); }
5851

5952
/** Removes a listener from the notifier. If the thread already holds the lock,
6053
* then the listener is removed later, but as soon as possible.
6154
* Note: It is NOT guaranteed that the listener will not be executed again.
6255
* @param listener a listener that reacts on events
6356
*/
64-
public void removeListener(final T listener) {
65-
try {
66-
_lock.startWrite();
67-
try { _listeners.remove(listener); }
68-
finally { _lock.endWrite(); }
69-
}
70-
catch(ReaderWriterLock.DeadlockException e) {
71-
// couldn't remove right now because this thread already owns a lock
72-
// remember to remove it later
73-
new Thread(new Runnable() {
74-
public void run() {
75-
_lock.startWrite();
76-
try { _listeners.remove(listener); }
77-
finally { _lock.endWrite(); }
78-
}
79-
}, "Pending Listener Removal").start();
80-
// synchronized(_listenersToRemove) {
81-
// _listenersToRemove.add(listener);
82-
// }
83-
}
84-
}
57+
public void removeListener(final T listener) { _listeners.remove(listener); }
8558

8659
/** Removes all listeners from this notifier. If the thread already holds the lock,
8760
* then the listener is removed later, but as soon as possible.
8861
* Note: It is NOT guaranteed that the listener will not be executed again. */
89-
public void removeAllListeners() {
90-
try {
91-
_lock.startWrite();
92-
try { _listeners.clear(); }
93-
finally { _lock.endWrite(); }
94-
}
95-
catch(ReaderWriterLock.DeadlockException e) {
96-
// couldn't remove right now because this thread already owns a lock
97-
// remember to remove it later
98-
new Thread(new Runnable() {
99-
public void run() {
100-
_lock.startWrite();
101-
try { _listeners.clear(); }
102-
finally { _lock.endWrite(); }
103-
}
104-
}, "Pending Listener Removal").start();
105-
}
106-
}
62+
public void removeAllListeners() { _listeners.clear(); }
10763
}

0 commit comments

Comments
 (0)