Semaphore clarification

[pjc30943]

EDIT: corrected a basic logic error, but the error still occurs
================================================

This is now in a separate thread. Regarding semaphore usage, what's wrong with the following code? [The code should be complete, minus copy/paste omissions or errors.]

Code: Select all

public DACsem as boolean

public const task1stackSize as integer = 100
public task1stack (1 to task1stackSize  ) as byte

public const task2stackSize as integer = 100
public task2stack (1 to task2stackSize  ) as byte


Sub Main()
	DACsem = false
	calltask task1, task1stack
	calltask task2, task2stack
end sub


sub task1()
	do 
		do until ( Semaphore(DACsem) = true)
			sleep 0.0
		loop
		putDACs 1.0
		DACsem = false			
		sleep 0.0
	loop
end sub


sub task2()
'task2 is an exact copy of task1.  Task1 could have been called twice with different parameters passed to them for task IDs...
	do 
		do until ( Semaphore(DACsem) = true)
			sleep 0.0
		loop
		putDACs 2.0
		DACsem = false			
		sleep 0.0
	loop
end sub

sub putDACs(byval V as single) 
   'The " from 1.0" or "from 2.0" are displayed to make it possible to know which task called this routine.
   debug.print 
   debug.print "O from ";cstr(V); ", DACsem="; cstr(DACsem)
   sleep 0.5
   debug.print "F from ";cstr(V); ", DACsem="; cstr(DACsem)
   sleep 0.5
end sub

Output should only have groups of Oi / Fi, since the putDACs routine should be locked until completion.

...

O from 1.0, DACsem= true
F from 1.0, DACsem= true

O from 2.0, DACsem= true
F from 2.0, DACsem= true
...

or in the reverse order.
But never like:

...
O from 2.0, DACsem= true

O from 1.0, DACsem= true
F from 2.0, DACsem= true
F from 1.0, DACsem= true
...

which is what is actually output, or some garbled combination thereof, since the routine is being called again before completion. Most likely this is a really silly error. Could anyone please correct this implementation of semaphores?

[dkinzer]

EDIT: corrected a basic logic error, but the error still occurs

I went to post a response indicating that the semaphore acquisition logic was wrong but you fixed it before I could.

The modified program works correctly here, tested on a ZX-24a but using a later version of the compiler. Perhaps you could zip up the listing and .zxb file and email it to me.

[mikep]

Could anyone please correct this implementation of semaphores?

Your full example code looks very similar to what I tried last night. I have now also tried your example code and still cannot get it to fail. I tried it with ZVM version 2.3 on ZX24e with compiler version 2.2.

The versions of what you are using may be helpful to Don as is the ZXB file he requested. On the other hand I don't ever recall a problem in this functional area that might indicate that you have a older (and non-working) version of either the VM or the compiler.

This is only a question of problem isolation.

[pjc30943]

Thanks Don and Mike. Here is a zipped folder with the asked details. An example output is shown.

This is undoubtedly something quite simple and basic that I've failed to implement properly.

[mikep]

This is undoubtedly something quite simple and basic that I've failed to implement properly.

The good news is that I can reproduce this problem on a ZX-1281e, ZX-128e, and ZX-24e. The bad news is I don't yet know why as the ZVM code is mostly the same across devices. Stay tuned.

[mikep]

The line in error is as follows:

Code: Select all

sub waitForSem(byval sem as boolean)

This routine has a copy of the semphore flag on the stack. Therefore testing this flag is not the same as testing the actual semaphore. The correct code is to pass the semaphore by reference as follows:

Code: Select all

sub waitForSem(byref sem as boolean)

We were not able to reproduce this problem before because you didn't mention (or post) this vital piece of code.

[pjc30943]

The line in error is as follows:

Code: Select all

sub waitForSem(byval sem as boolean)

This routine has a copy of the semphore flag on the stack. Therefore testing this flag is not the same as testing the actual semaphore. The correct code is to pass the semaphore by reference as follows:

Code: Select all

sub waitForSem(byref sem as boolean)

We were not able to reproduce this problem before because you didn't mention (or post) this vital piece of code.

Ah! Thank you Mike.
You are right; the nature of Semaphore() requires passing by reference.
Originally the code did not have a separate sub waitForSem(..), but local loops as shown previously, which is why I didn't mention it.

I'm going to try your correction now. If it does work, as it should, then I'm sorry to have taken your time.

[dkinzer]

Originally the code did not have a separate sub waitForSem(..), but local loops as shown previously.

As a separate issue, if your code has multiple task routines that are nearly identical as in the example, you might consider using parameterized tasks. The parameterized equivalent might look like this:

Code: Select all

Sub Main
  DACsem = false
  CallTask task(1.0), task1stack
  CallTask task(2.0), task2stack
End Sub

Sub task(ByVal id as Single)
   Do
      Do Until (Semaphore(DACsem) = True)
        Call Sleep(0.0)
      Loop
      Call putDACs(id)
      DACsem = False
      Call Sleep(0.0)
   Loop
End Sub

[pjc30943]

Thanks Don. A statement in the code posted above contains this comment as well, and perhaps I should have done so originally for transparency.

I'm still seeing interference, where a delay (such as a debug.print statement in Common() ) is required for proper operation.
But this is no longer a semaphore issue, as they seem to work fine.

Originally the code did not have a separate sub waitForSem(..), but local loops as shown previously.

As a separate issue, if your code has multiple task routines that are nearly identical as in the example, you might consider using parameterized tasks. The parameterized equivalent might look like this:

Code: Select all

Sub Main
  DACsem = false
  CallTask task(1.0), task1stack
  CallTask task(2.0), task2stack
End Sub

Sub task(ByVal id as Single)
   Do
      Do Until (Semaphore(DACsem) = True)
        Call Sleep(0.0)
      Loop
      Call putDACs(id)
      DACsem = False
      Call Sleep(0.0)
   Loop
End Sub

[stevech]

Does ZBasic guarantee that
Call Sleep(0.0)

will put the task on the tail of the ready task queue? I would code this with some other procedure that relies on an assured characteristic of the VM system. I suppose sleeping one clock tick would do, or there may be some other scheduler call that would work.

[mikep]

Does ZBasic guarantee that
Call Sleep(0.0)

will put the task on the tail of the ready task queue?

The simple answer is yes as far as I understand it. The tasks are stored in RAM in a linked list. The scheduler is simple round-robin unless preempted by a task waiting on an interrupt or an interval.

After a sleep or delay, the current task pointer is moved to the "next" ready-to-run task which might result in the same task running again. Every timer tick the task list is searched and the task sleep count is decremented until it reaches zero and the task is again ready-to-run.

The task control block is described in section 3.24 of the current language reference manual.

[dkinzer]

Does ZBasic guarantee that Call Sleep(0.0) will put the task on the tail of the ready task queue?

Although there isn't a "ready task queue" it sort of works that way.

The primary multi-tasking data structure is a circular linked list containing all of the active Task Control Blocks. Invoking Sleep() with a zero parameter causes the task manager to look for the next task that is ready to run. It does this by scanning the task list, beginning with the entry following the current task, looking for a task that is ready to run. This same action occurs when a task is suspended for any reason, including reaching the end of its time slice.

[stevech]

My intended point was that if the code depends on forcing a task yield to another task, by design, then sleep(0), it seems, may not be a reliable way to do that. Better to use some API call that is documented for the purpose of doing a yield.

[dkinzer]

sleep(0), it seems, may not be a reliable way to do that.

Just to be clear, Sleep(0) means "yield to some other task that is ready to run". In contrast, RunTask() allows you to yield to a specific task. In both cases, the yielding task is not run again until its turn comes up in the normal task rotation or it is explicitly activated by some other means.

The following events may cause tasks to run in other than their normal, round-robin order:

• an expiring interval with WaitForInterval() active
• an external interrupt with WaitForInterrupt() active
• RunTask()

[stevech]

sleep(0), it seems, may not be a reliable way to do that.

Just to be clear, Sleep(0) means "yield to some other task that is ready to run". In contrast, RunTask() allows you to yield to a specific task. In both cases, the yielding task is not run again until its turn comes up in the normal task rotation or it is explicitly activated by some other means.

The following events may cause tasks to run in other than their normal, round-robin order:

• an expiring interval with WaitForInterval() active
• an external interrupt with WaitForInterrupt() active
• RunTask()

Right. Sleep(0) implies a yield to another task in ZBasic. But it's an internal implementation issue. Better might be an API clearly defined to yield and schedule the task to run *after* all (same-priority) tasks have run. An API call such as "yield()" or some such.

But sleep(0), documented as assuredly doing a yield, is fine I suppose.