Unlock memory and registers when blocked for user IO

This change was made to fix #48 This required changing from using synchronized blocks to explicitly using a reentrant lock. Everywhere there was a synchronization block there is now a call to lock() and a try finally with an unlock(). The refactoring I did in the parent was ultimately unneeded as I ended up making a special unlocker in the blocking methods the system calls use.
TheThirdOne · Sep 26, 2019 · 43652d8 · 43652d8
1 parent 7f6b0d3
commit 43652d8
Show file tree

Hide file tree

Showing 12 changed files with 60 additions and 13 deletions.
diff --git a/rars/Globals.java b/rars/Globals.java
@@ -11,6 +11,7 @@
 import java.util.Enumeration;
 import java.util.Properties;
 import java.util.StringTokenizer;
+import java.util.concurrent.locks.ReentrantLock;
 
 /*
 Copyright (c) 2003-2008,  Pete Sanderson and Kenneth Vollmar
@@ -70,7 +71,7 @@ public class Globals {
     /**
      * Lock variable used at head of synchronized block to guard memory and registers
      **/
-    public static final Object memoryAndRegistersLock = new Object();
+    public static final ReentrantLock memoryAndRegistersLock = new ReentrantLock();
     /**
      * Flag to determine whether or not to produce internal debugging information.
      **/

diff --git a/rars/riscv/syscalls/SyscallReadInt.java b/rars/riscv/syscalls/SyscallReadInt.java
@@ -1,6 +1,7 @@
 package rars.riscv.syscalls;
 
 import rars.ExitingException;
+import rars.Globals;
 import rars.ProgramStatement;
 import rars.riscv.AbstractSyscall;
 import rars.riscv.hardware.RegisterFile;

diff --git a/rars/simulator/Simulator.java b/rars/simulator/Simulator.java
@@ -390,7 +390,8 @@ public void run() {
                 // to access memory and registers only through synchronized blocks on same
                 // lock variable, then full (albeit heavy-handed) protection of memory and
                 // registers is assured.  Not as critical for reading from those resources.
-                synchronized (Globals.memoryAndRegistersLock) {
+                Globals.memoryAndRegistersLock.lock();
+                try {
                     // Handle pending interupts and traps first
                     int uip = ControlAndStatusRegisterFile.getValueNoNotify("uip"), uie = ControlAndStatusRegisterFile.getValueNoNotify("uie");
                     boolean IE = (ControlAndStatusRegisterFile.getValueNoNotify("ustatus") & ControlAndStatusRegisterFile.INTERRUPT_ENABLE) != 0;
@@ -514,7 +515,9 @@ public void run() {
                             return;
                         }
                     }
-                }// end synchronized block
+                } finally {
+                    Globals.memoryAndRegistersLock.unlock();
+                }
 
                 // Update cycle(h) and instret(h)
                 int cycle = ControlAndStatusRegisterFile.getValueNoNotify("cycle"),

diff --git a/rars/tools/AbstractToolAndApplication.java b/rars/tools/AbstractToolAndApplication.java
@@ -653,15 +653,21 @@ public ConnectButton() {
 
         public void connect() {
             observing = true;
-            synchronized (Globals.memoryAndRegistersLock) {// DPS 23 July 2008
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 addAsObserver();
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             setText(disconnectText);
         }
 
         public void disconnect() {
-            synchronized (Globals.memoryAndRegistersLock) {// DPS 23 July 2008
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 deleteAsObserver();
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             observing = false;
             setText(connectText);

diff --git a/rars/tools/DigitalLabSim.java b/rars/tools/DigitalLabSim.java
@@ -108,13 +108,16 @@ protected JComponent buildMainDisplayArea() {
 
     private synchronized void updateMMIOControlAndData(int dataAddr, int dataValue) {
         if (!this.isBeingUsedAsATool || (this.isBeingUsedAsATool && connectButton.isConnected())) {
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 try {
                     Globals.memory.setByte(dataAddr, dataValue);
                 } catch (AddressErrorException aee) {
                     System.out.println("Tool author specified incorrect MMIO address!" + aee);
                     System.exit(0);
                 }
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             if (Globals.getGui() != null && Globals.getGui().getMainPane().getExecutePane().getTextSegmentWindow().getCodeHighlighting()) {
                 Globals.getGui().getMainPane().getExecutePane().getDataSegmentWindow().updateValues();

diff --git a/rars/tools/FloatRepresentation.java b/rars/tools/FloatRepresentation.java
@@ -388,8 +388,11 @@ public void actionPerformed(ActionEvent e) {
     // If display is attached to a register then update the register value.
     private synchronized void updateAnyAttachedRegister(int intValue) {
         if (attachedRegister != null) {
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 attachedRegister.setValue(intValue);
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             // HERE'S A HACK!!  Want to immediately display the updated register value in RARS
             // but that code was not written for event-driven update (e.g. Observer) --

diff --git a/rars/tools/KeyboardAndDisplaySimulator.java b/rars/tools/KeyboardAndDisplaySimulator.java
@@ -677,14 +677,17 @@ private void updateMMIOControlAndData(int controlAddr, int controlValue, int dat
     // NOTE: last argument TRUE means update only the MMIO Control register; FALSE means update both Control and Data.
     private synchronized void updateMMIOControlAndData(int controlAddr, int controlValue, int dataAddr, int dataValue, boolean controlOnly) {
         if (!this.isBeingUsedAsATool || (this.isBeingUsedAsATool && connectButton.isConnected())) {
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 try {
                     Globals.memory.setRawWord(controlAddr, controlValue);
                     if (!controlOnly) Globals.memory.setRawWord(dataAddr, dataValue);
                 } catch (AddressErrorException aee) {
                     System.out.println("Tool author specified incorrect MMIO address!" + aee);
                     System.exit(0);
                 }
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             // HERE'S A HACK!!  Want to immediately display the updated memory value in MARS
             // but that code was not written for event-driven update (e.g. Observer) --

diff --git a/rars/tools/TimerTool.java b/rars/tools/TimerTool.java
@@ -277,13 +277,16 @@ public void reset() {
 
     // Writes a word to a virtual memory address
     private synchronized void updateMMIOControlAndData(int dataAddr, int dataValue) {
-        synchronized (Globals.memoryAndRegistersLock) {
+        Globals.memoryAndRegistersLock.lock();
+        try {
             try {
                 Globals.memory.setRawWord(dataAddr, dataValue);
             } catch (AddressErrorException aee) {
                 System.out.println("Tool author specified incorrect MMIO address!" + aee);
                 System.exit(0);
             }
+        } finally {
+            Globals.memoryAndRegistersLock.unlock();
         }
     }
 

diff --git a/rars/venus/DataSegmentWindow.java b/rars/venus/DataSegmentWindow.java
@@ -937,7 +937,8 @@ public void setValueAt(Object value, int row, int col) {
             }
             //  Assures that if changed during MIPS program execution, the update will
             //  occur only between instructions.
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 try {
                     Globals.memory.setRawWord(address, val);
                 }
@@ -946,6 +947,8 @@ public void setValueAt(Object value, int row, int col) {
                 catch (AddressErrorException aee) {
                     return;
                 }
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }// end synchronized block
             int valueBase = Globals.getGui().getMainPane().getExecutePane().getValueDisplayBase();
             data[row][col] = NumberDisplayBaseChooser.formatNumber(val, valueBase);

diff --git a/rars/venus/MessagesPane.java b/rars/venus/MessagesPane.java
@@ -353,12 +353,19 @@ public void selectRunMessageTab() {
      */
     public String getInputString(String prompt) {
         String input;
+        boolean lock = Globals.memoryAndRegistersLock.isHeldByCurrentThread();
+        if (lock) {
+            Globals.memoryAndRegistersLock.unlock();
+        }
         JOptionPane pane = new JOptionPane(prompt, JOptionPane.QUESTION_MESSAGE, JOptionPane.DEFAULT_OPTION);
         pane.setWantsInput(true);
         JDialog dialog = pane.createDialog(Globals.getGui(), "Keyboard Input");
         dialog.setVisible(true);
         input = (String) pane.getInputValue();
         this.postRunMessage(Globals.userInputAlert + input + "\n");
+        if (lock) {
+            Globals.memoryAndRegistersLock.lock();
+        }
         return input;
     }
 
@@ -376,8 +383,16 @@ public String getInputString(String prompt) {
      * @return User input.
      */
     public String getInputString(int maxLen) {
+        boolean lock = Globals.memoryAndRegistersLock.isHeldByCurrentThread();
+        if (lock) {
+            Globals.memoryAndRegistersLock.unlock();
+        }
         Asker asker = new Asker(maxLen); // Asker defined immediately below.
-        return asker.response();
+        String out = asker.response();
+        if (lock) {
+            Globals.memoryAndRegistersLock.lock();
+        }
+        return out;
     }
 
     ////////////////////////////////////////////////////////////////////////////

diff --git a/rars/venus/TextSegmentWindow.java b/rars/venus/TextSegmentWindow.java
@@ -731,7 +731,8 @@ public void setValueAt(Object value, int row, int col) {
             }
             //  Assures that if changed during MIPS program execution, the update will
             //  occur only between instructions.
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 try {
                     Globals.memory.setRawWord(address, val);
                 }
@@ -740,6 +741,8 @@ public void setValueAt(Object value, int row, int col) {
                 catch (AddressErrorException aee) {
                     return;
                 }
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }// end synchronized block
         }
 

diff --git a/rars/venus/registers/RegisterBlockWindow.java b/rars/venus/registers/RegisterBlockWindow.java
@@ -306,8 +306,11 @@ public void setValueAt(Object value, int row, int col) {
             }
             //  Assures that if changed during program execution, the update will
             //  occur only between instructions.
-            synchronized (Globals.memoryAndRegistersLock) {
+            Globals.memoryAndRegistersLock.lock();
+            try {
                 registers[row].setValue(val);
+            } finally {
+                Globals.memoryAndRegistersLock.unlock();
             }
             int valueBase = Globals.getGui().getMainPane().getExecutePane().getValueDisplayBase();
             data[row][col] = NumberDisplayBaseChooser.formatNumber(val, valueBase);