Re: [Help-smalltalk] [bug] Delay issue

[Gwenaël Casaccio]

On 01/02/2012 08:26, Paolo Bonzini wrote:
> On 01/31/2012 09:32 PM, Gwenaël Casaccio wrote:
> > Eval [
> >      | d1 d2 p1 p2 |
> >      sem := Semaphore new.
> >      d1 := Delay forSeconds: 1.
> >      p1 := [ d1 value: [ sem signal ] onTimeoutDo: [ ] ] fork.
> >      sem wait.
> >
> >      'value:onTimeoutDo:' displayNl.
> >      d1 := Delay forMilliseconds: 100.
> >      d1 value: [ [ true ] whileTrue ] onTimeoutDo: [ ].
> > ]
>
> Let's fix this too, then. :)
>
> Paolo
Hi Paolo,  the bug comes when the process is removed from the middle of 
the list.

So here is the patch

diff --git a/libgst/interp.c b/libgst/interp.c
index fe154be..515f84a 100644
--- a/libgst/interp.c
+++ b/libgst/interp.c
@@ -305,6 +305,9 @@ static OOP highest_priority_process (void);
    LinkedList) and answer it.  */
 static OOP remove_first_link (OOP semaphoreOOP);

+/* Remove a process of the given list */
+static void remove_from_link (OOP processOOP);
+
 /* Add PROCESSOOP as the head of the given list (a Semaphore is a
    subclass of LinkedList) and answer it.  */
 static void add_first_link (OOP semaphoreOOP,
@@ -1398,8 +1401,7 @@ get_scheduled_process (void)
 }

 void
-add_first_link (OOP semaphoreOOP,
-               OOP processOOP)
+remove_from_link (OOP processOOP)
 {
   gst_semaphore sem;
   gst_process process, lastProcess;
@@ -1410,28 +1412,40 @@ add_first_link (OOP semaphoreOOP,
     {
       sem = (gst_semaphore) OOP_TO_OBJ (process->myList);
       if (sem->firstLink == processOOP)
-       {
-         sem->firstLink = process->nextLink;
-         if (sem->lastLink == processOOP)
-           {
-             /* It was the only process in the list */
-             sem->lastLink = _gst_nil_oop;
-           }
-       }
-      else if (sem->lastLink == processOOP)
-       {
-         /* Find the new last link */
-         lastProcessOOP = sem->firstLink;
-         lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
-         while (lastProcess->nextLink != processOOP)
-           {
-             lastProcessOOP = lastProcess->nextLink;
-             lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
-           }
-         sem->lastLink = lastProcessOOP;
-         lastProcess->nextLink = _gst_nil_oop;
-       }
+        {
+          sem->firstLink = process->nextLink;
+          if (sem->lastLink == processOOP)
+            {
+              /* It was the only process in the list */
+              sem->lastLink = _gst_nil_oop;
+            }
+        }
+      else
+        {
+          /* Find the new middle link */
+          lastProcessOOP = sem->firstLink;
+          lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
+          while (lastProcess->nextLink != processOOP)
+            {
+              lastProcessOOP = lastProcess->nextLink;
+              lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
+            }
+          if (sem->lastLink == processOOP)
+            sem->lastLink = lastProcessOOP;
+          lastProcess->nextLink = process->nextLink;
+        }
     }
+}
+
+void
+add_first_link (OOP semaphoreOOP,
+               OOP processOOP)
+{
+  gst_semaphore sem;
+  gst_process process;
+
+  process = (gst_process) OOP_TO_OBJ (processOOP);
+  remove_from_link (processOOP);

   sem = (gst_semaphore) OOP_TO_OBJ (semaphoreOOP);
   process->myList = semaphoreOOP;
@@ -1509,36 +1523,10 @@ add_last_link (OOP semaphoreOOP,
               OOP processOOP)
 {
   gst_semaphore sem;
-  gst_process process, lastProcess;
-  OOP lastProcessOOP;
+  gst_process process;

   process = (gst_process) OOP_TO_OBJ (processOOP);
-  if (!IS_NIL (process->myList))
-    {
-      sem = (gst_semaphore) OOP_TO_OBJ (process->myList);
-      if (sem->firstLink == processOOP)
-       {
-         sem->firstLink = process->nextLink;
-         if (sem->lastLink == processOOP)
-           {
-             /* It was the only process in the list */
-             sem->lastLink = _gst_nil_oop;
-           }
-       }
-      else if (sem->lastLink == processOOP)
-       {
-         /* Find the new last link */
-         lastProcessOOP = sem->firstLink;
-         lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
-         while (lastProcess->nextLink != processOOP)
-           {
-             lastProcessOOP = lastProcess->nextLink;
-             lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
-           }
-         sem->lastLink = lastProcessOOP;
-         lastProcess->nextLink = _gst_nil_oop;
-       }
-    }
+  remove_from_link (processOOP);

   sem = (gst_semaphore) OOP_TO_OBJ (semaphoreOOP);
   process->myList = semaphoreOOP;
@@ -1548,6 +1536,9 @@ add_last_link (OOP semaphoreOOP,
     sem->firstLink = sem->lastLink = processOOP;
   else
     {
+      OOP lastProcessOOP;
+      gst_process lastProcess;
+
       lastProcessOOP = sem->lastLink;
       lastProcess = (gst_process) OOP_TO_OBJ (lastProcessOOP);
       lastProcess->nextLink = processOOP;
@@ -2082,8 +2073,10 @@ _gst_check_process_state (void)
            abort ();

          /* Check (rather brutally) for loops in the process lists.  */
-         if (++n>  _gst_mem.ot_size)
+         if (++n>  _gst_mem.ot_size) {
+           fprintf (stderr, "%d\n\n", priority);
            abort ();
+         }
        }
     }
 }