Skip to content

Commit 05e85ac

Browse files
ponderosaTXponderosaTX
committed
Minor revisions to swing/RMI synchronization to address lost test reporting in the JUnitPanel and a change in the switches passes to the javax.tools compiler (eliminating the -Xlint option).
Changes to be committed: deleted: ../../../../../../drjava.jar modified: model/EventNotifier.java modified: model/compiler/CompilerEventNotifier.java modified: model/compiler/JavaxToolsCompiler.java modified: model/definitions/ClassNameNotFoundException.java modified: model/javadoc/JavadocEventNotifier.java modified: model/junit/DefaultJUnitModel.java modified: model/junit/JUnitEventNotifier.java modified: model/repl/newjvm/MainJVM.java modified: ui/avail/GUIAvailabilityNotifier.java modified: ui/coverage/CoverageFrame.java
1 parent d86ca6f commit 05e85ac

File tree

11 files changed

+49
-90
lines changed

11 files changed

+49
-90
lines changed

‎drjava.jar‎

-10.5 MB
Binary file not shown.

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,25 @@
3030

3131
import java.util.List;
3232
import java.util.concurrent.CopyOnWriteArrayList;
33-
import edu.rice.cs.util.ReaderWriterLock;
3433

3534
/** Base class for all component-specific EventNotifiers. This class provides common methods to
3635
* manage listeners of a specific type. T the type of the listener class to be managed.
3736
* @version $Id$
3837
*/
3938
public abstract class EventNotifier<T> {
4039
/** All T Listeners that are listening to the model. Accesses to this collection are protected by the
41-
* ReaderWriterLock. The collection must be synchronized, since multiple readers could access it at once.
40+
* ReaderWriterLock. The collection relies on "copy-on-write" semantics for _listeners.
4241
*/
4342
protected final List<T> _listeners = new CopyOnWriteArrayList<T>();
4443

45-
/* The listener framework is now implemented using CopyOnWriteArrayList, eliminating the readers/writers issue. */
44+
/* Since the listener framework is now implemented using CopyOnWriteArrayList, no readers/writers locking is necessary. */
4645

47-
/** Adds a listener to the notifier.
46+
/** Adds a listener to this notifier.
4847
* @param listener a listener that reacts on events
4948
*/
5049
public void addListener(T listener) { _listeners.add(listener); }
5150

52-
/** Removes a listener from the notifier. If the thread already holds the lock,
51+
/** Removes a listener from this notifier. If the thread already holds the lock,
5352
* then the listener is removed later, but as soon as possible.
5453
* Note: It is NOT guaranteed that the listener will not be executed again.
5554
* @param listener a listener that reacts on events

‎drjava/src/edu/rice/cs/drjava/model/compiler/CompilerEventNotifier.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,6 @@
5151
* components, and should not be used directly outside of the "host" component.
5252
* <p>
5353
*
54-
* All methods in this class must use the synchronization methods
55-
* provided by ReaderWriterLock. This ensures that multiple notifications
56-
* (reads) can occur simultaneously, but only one thread can be adding
57-
* or removing listeners (writing) at a time, and no reads can occur
58-
* during a write.
59-
* <p>
60-
*
61-
* <i>No</i> methods on this class should be synchronized using traditional
62-
* Java synchronization!
63-
* <p>
64-
*
6554
* @version $Id$
6655
*/
6756
class CompilerEventNotifier extends EventNotifier<CompilerListener> implements CompilerListener {

‎drjava/src/edu/rice/cs/drjava/model/compiler/JavaxToolsCompiler.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,10 @@ public List<? extends DJError> compile(List<? extends File> files, List<? extend
6666
Iterable<? extends JavaFileObject> compilationUnits = fileManager.getJavaFileObjectsFromFiles(files);
6767

6868
// Prepare the compilation options
69-
/* Question (by Corky): is the "-source" option necessary? The JavaxTools compiler is part of the executing JVM. */
69+
/* Question (by Corky): is the "-source" option necessary? The JavaxTools compiler is part of the executing JVM.
70+
* All calls on compile appear to pass null as the sourceVersion. */
7071
List<String> optionList = new ArrayList<>();
71-
optionList.add("-Xlint");
72+
// optionList.add("-Xlint");
7273
if (sourceVersion != null) {
7374
optionList.add("-source");
7475
optionList.add(sourceVersion);

‎drjava/src/edu/rice/cs/drjava/model/definitions/ClassNameNotFoundException.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@
2828
* END_COPYRIGHT_BLOCK*/
2929
package edu.rice.cs.drjava.model.definitions;
3030

31-
/** * Exception indicating that a class name could not be found in
32-
* the DefinitionsDocument from which it was thrown.
33-
* @version $Id$
34-
*/
31+
/** Exception indicating that a class name could not be found in the DefinitionsDocument from which it was thrown.
32+
* @version $Id$
33+
*/
3534
public class ClassNameNotFoundException extends Exception {
3635

3736
/** Creats a new ClassNameNotFoundException with the given label. */

‎drjava/src/edu/rice/cs/drjava/model/javadoc/JavadocEventNotifier.java‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,9 @@
5050
* components, and should not be used directly outside of the "host" component.
5151
* <p>
5252
*
53-
* All methods in this class must use the synchronization methods
54-
* provided by ReaderWriterLock. This ensures that multiple notifications
55-
* (reads) can occur simultaneously, but only one thread can be adding
56-
* or removing listeners (writing) at a time, and no reads can occur
57-
* during a write.
58-
* <p>
59-
*
60-
* <i>No</i> methods on this class should be synchronized using traditional
61-
* Java synchronization!
53+
* All methods in this class should use the "copy-on-write" semantics of the _listeners colletion. This protocol
54+
* ensures that multiple notifications can occur simultaneously while other threads "atomically" modify the _listeners
55+
* collection (each using a single method of the CopyOnWriteArrayList class.
6256
* <p>
6357
*
6458
* @version $Id$

‎drjava/src/edu/rice/cs/drjava/model/junit/DefaultJUnitModel.java‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -527,10 +527,10 @@ private void _notifyJUnitEnded() {
527527
}
528528

529529
/** Helper method to notify JUnitModel listeners that all open files must be
530-
* compiled before JUnit is run.
531-
* @param testAfterCompile a CompilerListener
532-
* @param outOfSync list of out-of-sync documents
533-
*/
530+
* compiled before JUnit is run.
531+
* @param testAfterCompile a CompilerListener
532+
* @param outOfSync list of out-of-sync documents
533+
*/
534534
private void _notifyCompileBeforeJUnit(final CompilerListener testAfterCompile,
535535
final List<OpenDefinitionsDocument> outOfSync) {
536536
Utilities.invokeLater(new Runnable() {
@@ -539,10 +539,10 @@ private void _notifyCompileBeforeJUnit(final CompilerListener testAfterCompile,
539539
}
540540

541541
/** Helper method to notify JUnitModel listeners that JUnit aborted before
542-
* any tests could be run.
543-
* @param testAll true if all tests are to be run
544-
* @param didCompileFail true if compilation failed
545-
*/
542+
* any tests could be run.
543+
* @param testAll true if all tests are to be run
544+
* @param didCompileFail true if compilation failed
545+
*/
546546
private void _notifyNonTestCase(final boolean testAll, final boolean didCompileFail) {
547547
Utilities.invokeLater(new Runnable() { public void run() { _notifier.nonTestCase(testAll, didCompileFail); } });
548548
}
@@ -604,29 +604,25 @@ public void testEnded(final String testName, final boolean wasSuccessful, final
604604
public void testSuiteEnded(final JUnitError[] errors) {
605605
// new ScrollableDialog(null, "DefaultJUnitModel.testSuiteEnded(...) called", "", "").show();
606606

607-
Utilities.invokeLater(new Runnable() {
608-
public void run() {
609-
List<File> files = new ArrayList<File>();
610-
for(OpenDefinitionsDocument odd: _model.getLLOpenDefinitionsDocuments()) { files.add(odd.getRawFile()); }
611-
// Utilities.show("errors.length = " + errors.length + " files = " + files);
612-
for(JUnitError e: errors) {
613-
try {
614-
e.setStackTrace(_compilerModel.getLLSTM().replaceStackTrace(e.stackTrace(),files));
615-
} catch(Exception ex) { DrJavaErrorHandler.record(ex); }
616-
File f = e.file();
617-
if ((f != null) && (DrJavaFileUtils.isLLFile(f))) {
618-
String dn = DrJavaFileUtils.getJavaForLLFile(f.getName());
619-
StackTraceElement ste = new StackTraceElement(e.className(), "", dn, e.lineNumber());
620-
ste = _compilerModel.getLLSTM().replaceStackTraceElement(ste, f);
621-
e.setLineNumber(ste.getLineNumber());
622-
}
623-
}
624-
_junitErrorModel = new JUnitErrorModel(errors, _model, true);
625-
_notifyJUnitEnded();
626-
_testInProgress = false;
627-
// new ScrollableDialog(null, "DefaultJUnitModel.testSuiteEnded(...) finished", "", "").show();
607+
final List<File> files = new ArrayList<File>();
608+
for(OpenDefinitionsDocument odd: _model.getLLOpenDefinitionsDocuments()) { files.add(odd.getRawFile()); }
609+
for(JUnitError e: errors) {
610+
try {
611+
e.setStackTrace(_compilerModel.getLLSTM().replaceStackTrace(e.stackTrace(),files));
612+
} catch(Exception ex) { DrJavaErrorHandler.record(ex); }
613+
File f = e.file();
614+
if ((f != null) && (DrJavaFileUtils.isLLFile(f))) {
615+
String dn = DrJavaFileUtils.getJavaForLLFile(f.getName());
616+
StackTraceElement ste = new StackTraceElement(e.className(), "", dn, e.lineNumber());
617+
ste = _compilerModel.getLLSTM().replaceStackTraceElement(ste, f);
618+
e.setLineNumber(ste.getLineNumber());
628619
}
629-
});
620+
}
621+
_junitErrorModel = new JUnitErrorModel(errors, _model, true);
622+
_notifyJUnitEnded();
623+
_testInProgress = false;
624+
625+
// new ScrollableDialog(null, "DefaultJUnitModel.testSuiteEnded(...) finished", "", "").show();
630626
}
631627

632628

‎drjava/src/edu/rice/cs/drjava/model/junit/JUnitEventNotifier.java‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,28 +46,15 @@
4646
* interface.
4747
* <p>
4848
*
49-
* Components which might otherwise manage their own list of listeners use
49+
* Components which might otherwise manage their own list of listeners extend
5050
* EventNotifiers instead to simplify their internal implementation. Notifiers
5151
* should therefore be considered a private implementation detail of the
5252
* components, and should not be used directly outside of the "host" component.
5353
* <p>
5454
*
55-
* All methods in this class must use the synchronization methods
56-
* provided by ReaderWriterLock. This ensures that multiple notifications
57-
* (reads) can occur simultaneously, but only one thread can be adding
58-
* or removing listeners (writing) at a time, and no reads can occur
59-
* during a write.
60-
* <p>
61-
* Addendum [Corky March, 2025] In hindsight, the event notification framework is unnecessarily complex and buggy. Essentially
62-
* all event notification code runs in the "dispatch (event-handling) thread". The design should have forced ALL event
63-
* notication code to run in the dispatch thread. Then the read-write locking protocol would be unnecessary. I suspect
64-
* that "adminstrative methods" like addListener and removeListener are accessed from outside of the dispatch thread. In addition,
65-
* some events are signalled by RMI calls from JUnitTestRunner (and elsewhere?) in the slave JVM. The RMI "proxy" thead in
66-
* the main JVM apparently does not route event notifications through the dispatch thread. Ugh.
67-
* <p>
68-
* <i>No</i> methods on this class should be synchronized using traditionalJava synchronization!
69-
* <p>
70-
*
55+
* All methods in this class that manipulate the _listeners collection should rely on the "copy-on-write"
56+
* semantics for operations that mutate the collection. This protocol ensures that multiple notifications
57+
* (reads) can occur simultaneously while other threads are modifying the _listeners collection.
7158
* @version $Id$
7259
*/
7360
class JUnitEventNotifier extends EventNotifier<JUnitListener> implements JUnitListener {

‎drjava/src/edu/rice/cs/drjava/model/repl/newjvm/MainJVM.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,9 @@ public void run() {
262262
}
263263

264264
/** Called to indicate that a suite of tests has started running.
265-
* Forwards from the other JVM to the local JUnit model.
266-
* @param numTests The number of tests in the suite to be run.
267-
*/
265+
* Forwards from the other JVM to the local JUnit model.
266+
* @param numTests The number of tests in the suite to be run.
267+
*/
268268
public void testSuiteStarted(int numTests) {
269269
_junitModel.testSuiteStarted(numTests);
270270
}

‎drjava/src/edu/rice/cs/drjava/ui/avail/GUIAvailabilityNotifier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class GUIAvailabilityNotifier extends EventNotifier<GUIAvailabilityListen
5050

5151
// public static final edu.rice.cs.util.Log LOG = new edu.rice.cs.util.Log("avail.txt",true);
5252

53-
/** Create a new notifier with all components available. */
53+
/** Standard constructor. Creates a new notifier with all components available. */
5454
public GUIAvailabilityNotifier() {
5555
for(ComponentType component: ComponentType.values()) {
5656
_values.put(component, 0);

0 commit comments

Comments
 (0)