JavaMonitor.st
changeset 3773 f2fdd527603f
parent 3431 82790b1e6d54
child 3726 b133650e5820
child 3777 971961e59d5e
--- a/JavaMonitor.st	Thu May 04 23:54:12 2017 +0200
+++ b/JavaMonitor.st	Tue Feb 07 13:48:18 2017 +0100
@@ -1,5 +1,3 @@
-"{ Encoding: utf8 }"
-
 "
  COPYRIGHT (c) 1996-2015 by Claus Gittinger
 
@@ -119,9 +117,25 @@
 !
 
 decrement
-    "owning process released monitor, lets lower our counter so we know how many times
-     do we have to release it"
+    "owning process released monitor, let's lower our counter so we know how many times
+     we have to release it"
     
+    "/ the lock is probably not helping here to protect against problems with
+    "/ increment/decrement:
+    "/ the write-access (in reinitCOunter) will be either done before incrementing/decrementing or after.
+    "/ if after, everything is ok;
+    "/ if before, the decremented value will be zero, which is probably wrong.
+
+    "/ without the critical region, we might also get possible inconsistencies:
+    "/ if written before, the counter will drop to zero
+    "/ if written in-between, the counter wil ahve the old value decremented;
+    "/ if written afterwards, everything is ok.
+
+    "/ so, the lock should be really elsewhere, making sure that noone will count
+    "/ the monitor at-all and the monitor is reininialized without anyone depending on the
+    "/ count at all.
+    "/ but then, no critical section is needed in reinitCounter!!.
+
     instVarAccess critical: [ count := count - 1 ].
 
     "Created: / 22-11-2011 / 10:49:33 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
@@ -131,15 +145,19 @@
     JavaVM monitorTrace ifTrue:[
         Logger log: ('Waiting is disabled on monitor for %1' bindWith: ownerPrintString) severity:Logger severityDEBUG facility:#JVM.
     ].
-    instVarAccess critical: [ waitEnabled := false ].
+    "/ critical region not needed here
+    "/ instVarAccess critical: [ waitEnabled := false ].
+    waitEnabled := false.
 
     "Created: / 30-11-2011 / 20:34:40 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
     "Modified: / 02-03-2015 / 14:06:51 / Jan Vrany <jan.vrany@fit.cvut.cz>"
 !
 
 enableWait
-    instVarAccess critical: [waitEnabled := true].
-
+    "/ critical region not needed here
+    "/ instVarAccess critical: [waitEnabled := true].
+    waitEnabled := true
+    
     "Created: / 30-11-2011 / 20:34:31 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
     "Modified (format): / 11-10-2013 / 11:17:58 / Jan Vrany <jan.vrany@fit.cvut.cz>"
 !
@@ -148,6 +166,22 @@
     "owning process entered monitor again (recursion ...), lets raise our counter so we know how many times
      do we have to release it"
     
+    "/ the lock is probably not helping here to protect against problems with
+    "/ increment/decrement:
+    "/ the write-access (in reinitCOunter) will be either done before incrementing/decrementing or after.
+    "/ if after, everything is ok;
+    "/ if before, the decremented value will be zero, which is probably wrong.
+
+    "/ without the critical region, we might also get possible inconsistencies:
+    "/ if written before, the counter will drop to zero
+    "/ if written in-between, the counter wil ahve the old value decremented;
+    "/ if written afterwards, everything is ok.
+
+    "/ so, the lock should be really elsewhere, making sure that noone will count
+    "/ the monitor at-all and the monitor is reininialized without anyone depending on the
+    "/ count at all.
+    "/ but then, no critical section is needed in reinitCounter!!.
+
     instVarAccess critical: [ count := count + 1 ].
 
     "Created: / 22-11-2011 / 10:49:07 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
@@ -183,6 +217,22 @@
 
 reinitCounter
     "owning process is different from previous, lets start counting from beginning"
+
+    "/ the lock is probably not helping here to protect against problems with
+    "/ increment/decrement:
+    "/ the write-access here will be either done before incrementing/decrementing or after.
+    "/ if after, everything is ok;
+    "/ if before, the decremented value will be zero, which is probably wrong.
+
+    "/ without the critical region, we might also get possible inconsistencies:
+    "/ if written before, the counter will drop to zero
+    "/ if written in-between, the counter wil ahve the old value decremented;
+    "/ if written afterwards, everything is ok.
+    
+    "/ so, the lock should be really elsewhere, making sure that noone will count
+    "/ the monitor at-all and the monitor is reininialized without anyone depending on the
+    "/ count at all.
+    "/ but then, no critical section is needed!!.
     
     instVarAccess critical: [ count := 1 ].
 
@@ -190,6 +240,22 @@
 !
 
 reinitCounter: newCount 
+    "/ the lock is probably not helping here to protect against problems with
+    "/ increment/decrement:
+    "/ the write-access here will be either done before incrementing/decrementing or after.
+    "/ if after, everything is ok;
+    "/ if before, the decremented value will be zero, which is probably wrong.
+
+    "/ without the critical region, we might also get possible inconsistencies:
+    "/ if written before, the counter will drop to zero
+    "/ if written in-between, the counter wil ahve the old value decremented;
+    "/ if written afterwards, everything is ok.
+
+    "/ so, the lock should be really elsewhere, making sure that noone will count
+    "/ the monitor at-all and the monitor is reininialized without anyone depending on the
+    "/ count at all.
+    "/ but then, no critical section is needed!!.
+
     instVarAccess critical: [ count := newCount ].
 
     "Created: / 22-11-2011 / 13:00:51 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
@@ -206,7 +272,9 @@
 !
 
 waitEnabled
-    instVarAccess critical: [ ^ waitEnabled].
+    "/ critical region not needed here
+    "/ instVarAccess critical: [ ^ waitEnabled].
+    ^ waitEnabled.
 
     "Created: / 30-11-2011 / 20:34:56 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
 ! !
@@ -214,12 +282,14 @@
 !JavaMonitor methodsFor:'initialization'!
 
 initialize
+    "Q: is this ever called?"
+    
     owningProcess := nil.
     processesEntered := OrderedCollection new.
     monitorSema := Semaphore new: 1.
     processesWaiting := Dictionary new.
     waitingSema := Semaphore new: 0.
-    waitEnabled.
+    waitEnabled := true.
 
     "Created: / 20-11-2011 / 13:28:28 / Marcel Hlopko <hlopkmar@fel.cvut.cz>"
 !
@@ -435,7 +505,7 @@
 !JavaMonitor class methodsFor:'documentation'!
 
 version_CVS
-    ^ '$Header: /cvs/stx/stx/libjava/JavaMonitor.st,v 1.6 2015-03-20 12:08:00 vrany Exp $'
+    ^ '$Header$'
 !
 
 version_HG
@@ -444,7 +514,7 @@
 !
 
 version_SVN
-    ^ 'Id'
+    ^ '$Id$'
 ! !