Incorrect string value sometimes returned by thread?

[fluent]

When running this test I keep getting the same "INPUT" value multiple times... Any idea why?
I've tried using a Mutex but with little success.

Would love a short fix (even if quick and dirty )

Code: Select all

Result.s
global x = CreateMutex()

Procedure DoSomeWork(*x)
  
  Shared Result
  LockMutex(x)
  MM$ = peekS(*x) 
  UnLockMutex(x)
  
  delay(random(500))
  
  Result= MM$ + #TAB$ + str(random(100000))
  PostEvent(999)
  
EndProcedure   

Procedure.s LookUp(Input$)
  shared InputCopy$ 
  LockMutex(x)
  
  InputCopy$ = Input$
  UnLockMutex(x)
  
  CreateThread(@DoSomeWork(), @InputCopy$)
EndProcedure


OpenWindow(0, 100, 100, 640, 380, "")
EditorGadget(1, 1, 1, 600, 300)

for u = 1 to 10
  LookUp("INPUT "+str(random(10000)))
next 


Repeat
  e = WaitWindowEvent()
  Select e
    Case  999:  AddGadgetItem(1, -1,  Result) 
    Case  #PB_Event_CloseWindow : End
  EndSelect
Forever

[skywalk]

Did you compile thread safe?

[fluent]

Did you compile thread safe?

Yes...

This is the kind of result I am getting:
(Inputs are supposed to be random but I often see the same values repeated many times)

[skywalk]

Build each function without threading to verify your intent. Start with a timer if you want.
Use enableexplicit.
Drop shared.
Then use search for proper threading examples with postevent.
I'm not on a computer now.

[fluent]

Thanks, I will keep investigating.

Also here is a simplified code showing the same behaviour

Code: Select all

Result.s
InputCopy.s

Procedure DoSomeWork(*x)
  Shared Result.s

  MM$ = peekS(*x) 
  debug MM$
EndProcedure   

Procedure.s LookUp(Input.s)
  shared InputCopy.s 
  
  InputCopy.s = Input.s
  
  CreateThread(@DoSomeWork(), @InputCopy.s)
EndProcedure


OpenWindow(0, 100, 100, 640, 380, "")

for u = 1 to 10
  LookUp("INPUT "+str(random(100000)))
next 


Repeat : Until WaitWindowEvent() = #PB_Event_CloseWindow 

[RASHAD]

Dirty hack as you said

Code: Select all

For u = 1 To 10
  look$ = Str(Random(10000))
  Repeat
    Delay(1)
  Until look$ <> oldlook$
  oldlook$ = look$ 
  LookUp("INPUT "+look$)  
Next

[fluent]

Dirty hack as you said

Code: Select all

For u = 1 To 10
  look$ = Str(Random(10000))
  Repeat
    Delay(1)
  Until look$ <> oldlook$
  oldlook$ = look$ 
  LookUp("INPUT "+look$)  
Next

Thanks! And in the meantime I came up with a somewhat similar hack:

Code: Select all

Procedure DoSomeWork(*x)
  shared InputCopy.s 
  MM$ = peekS(*x) 
  InputCopy = ""
  
  ;; do work here
  
  debug "MM: " + MM$
EndProcedure   

Procedure.s LookUp(Input.s)
  shared InputCopy.s 
  
  while InputCopy <> "" : wend 
  InputCopy.s = Input.s
  
  CreateThread(@DoSomeWork(), @InputCopy.s)
EndProcedure

OpenWindow(0, 100, 100, 640, 380, "")

for u = 1 to 10
  LookUp("INPUT "+str(random(100000)))
next 

Repeat : Until WaitWindowEvent() = #PB_Event_CloseWindow 

[NicTheQuick]

The issue here ist that the string could be already a different one before the thread even reads it. You need to create a different string buffer for each thread before starting it.

Code: Select all

Procedure DoSomeWork(*x.String)

	;; do work here
	
	Debug "MM: " + *x\s
	
	FreeStructure(*x)
EndProcedure   

Procedure.s LookUp(Input.s)
	Protected *string.String = AllocateStructure(String)
	*string\s = Input
	
	CreateThread(@DoSomeWork(), *string)
EndProcedure

OpenWindow(0, 100, 100, 640, 380, "")

For u = 1 To 10
	LookUp("INPUT "+Str(Random(100000)))
Next

Repeat : Until WaitWindowEvent() = #PB_Event_CloseWindow 

[freak]

The reason for the behavior is that the thread does not immediately start running when you call CreateThread(). The CreateThread() call can return while the thread has not started yet or before the thread has reached the LockMutex() line. It is all asynchronous so there are no guarantees that things happen in a certain order between the threads.

So what happens here is that the LookUp() procedure is called multiple times before the created thread gets to pick up the value. Then the multiple created threads all try to read the value before the next LookUp() can update the value. Hence multiple same values picked up by the threads.

You can solve this like NicTheQuick said by using different buffers for each thread. If you do want to communicate through a single shared value, then you have to use a semaphore to communicate between the threads so the main thread can wait for the pickup to be done before moving on. Like this:

Code: Select all

Result.s
Global x = CreateMutex()
Global sem = CreateSemaphore()

Procedure DoSomeWork(*x)
 
  Shared Result
  LockMutex(x)
  MM$ = PeekS(*x)
  UnlockMutex(x)
  
  SignalSemaphore(sem) ; signal that the work has been picked up
 
  Delay(Random(500))
 
  Result= MM$ + #TAB$ + Str(Random(100000))
  PostEvent(999)
 
EndProcedure   

Procedure.s LookUp(Input$)
  Shared InputCopy$
  LockMutex(x)
 
  InputCopy$ = Input$
  UnlockMutex(x)
 
  CreateThread(@DoSomeWork(), @InputCopy$)
    
  WaitSemaphore(sem) ; wait for the work to be picked up
EndProcedure


OpenWindow(0, 100, 100, 640, 380, "")
EditorGadget(1, 1, 1, 600, 300)

For u = 1 To 10
  LookUp("INPUT "+Str(Random(10000)))
Next


Repeat
  e = WaitWindowEvent()
  Select e
    Case  999:  AddGadgetItem(1, -1,  Result)
    Case  #PB_Event_CloseWindow : End
  EndSelect
ForEver

Alternatively, depending on the size of the work to do it may be more efficient to work with a fixed set of threads and a queue so threads do not have to be created/destroyed all the time. There is an example for this in the documentation: https://www.purebasic.com/documentation ... phore.html


You actually have the same problem with the PostEvent() here as well. Multiple results can be posted to the main thread but you only have one variable to communicate the result. So the exact same thing can happen here too. Using a list or queue to hold multiple values would solve this.

[infratec]

You have to protect the overwrite of your global inputstring.

But with LockMutex it is not possible in your case, since
a mutex can only block different threads and not the same thread.
You would need LockMutex before you overwrite the inputstring.
But since this is in the same 'thread' it would not lock.

So you need a semaphore which is thread independent.

Code: Select all

EnableExplicit

Global Semaphore.i

Define InputCopy.s

Procedure DoSomeWork(*x)
  
  Protected MM$
  

  MM$ = PeekS(*x)
  SignalSemaphore(Semaphore)
  
  Debug "DSW: " + MM$
  
  Delay(3000)
  
  Debug "Thread finished"
  
EndProcedure   


Procedure.s LookUp(Input.s)
  
  Shared InputCopy.s
  
  
  InputCopy.s = Input.s  
  CreateThread(@DoSomeWork(), @InputCopy.s)
  WaitSemaphore(Semaphore)
  
EndProcedure


Define u.i

OpenWindow(0, 100, 100, 640, 380, "")

Semaphore = CreateSemaphore()

For u = 1 To 10
  LookUp("INPUT "+Str(Random(100000)))
Next


Repeat : Until WaitWindowEvent() = #PB_Event_CloseWindow 

[infratec]

To see that LockMutex does not work for the same thread:

Code: Select all

Define Mutex.i

Mutex = CreateMutex()
LockMutex(Mutex)
LockMutex(Mutex)
Debug 1

It does not lock in this case.

[NicTheQuick]

To see that LockMutex does not work for the same thread:

Code: Select all

Define Mutex.i

Mutex = CreateMutex()
LockMutex(Mutex)
LockMutex(Mutex)
Debug 1

It does not lock in this case.

That's because it is an reentrant lock. You actually have to call UnlockMutex() as often as you've called LockMutex() to actually unlock it. That is a feature which allows you to write functions or methods which by itself lock and unlock the mutex. And if the lock was already locked before you call that function it just works. Simple example:

Code: Select all

Global mutex = CreateMutex()

Procedure thread(dummy.i)
	LockMutex(mutex)
	Debug "Thread: Mutex locked"
	UnlockMutex(mutex)
EndProcedure

LockMutex(mutex)
Debug "Main: Mutex locked"
CreateThread(@thread(), 0)
Debug "Main: Thread started"
LockMutex(mutex)
Debug "Main: Mutex locked a second time."
UnlockMutex(mutex)
Debug "Main: Mutex unlocked."
UnlockMutex(mutex)
Debug "Main: Mutex unlocked again. Thread should start."
Delay(1000)
Debug "Main: end"

[mk-soft]

You can pass a string from thread to gui over PostEvent and EventData ...
Mutex not needed

Update: String to thread now save

Code: Select all

;-Top

CompilerIf Not #PB_Compiler_Thread
  CompilerError "Use Compiler Option ThreadSafe!"
CompilerEndIf

Enumeration CustomEvent #PB_Event_FirstCustomValue
  #MyEvent_ThreadString
EndEnumeration

Procedure AllocateString(String.s)
  Protected *mem.String
  *mem = AllocateStructure(String)
  If *mem
    *mem\s = String
  EndIf
  ProcedureReturn *mem
EndProcedure

Procedure.s FreeString(*mem.String)
  Protected r1.s
  If *mem
    r1 = *mem\s
    FreeStructure(*mem)
  EndIf
  ProcedureReturn r1
EndProcedure

; ----

Procedure thWork(*text)
  Protected r1.s
  Delay(100)
  r1 = FreeString(*text)
  r1 = "Revers: " + ReverseString(r1)
  PostEvent(#MyEvent_ThreadString, 0, 0, 0, AllocateString(r1))
EndProcedure

If OpenWindow(0, 50, 50, 400, 200, "Pass String from Thread to GUI", #PB_Window_SystemMenu)
  
  Define text.s = "Hello World!"
  *text = AllocateString(text)
  CreateThread(@thWork(), *text)
  text = ""
  Repeat
    Select WaitWindowEvent()
      Case #PB_Event_CloseWindow
        Break
        
      Case #MyEvent_ThreadString
        r1.s = FreeString(EventData())
        Debug "String from Thread: " + r1
        
    EndSelect
    
  ForEver
  
EndIf

[mk-soft]

Working with threads is easy. There are only a few rules to follow.

1 - Activate the compiler option Thread Safe to switch the internal function of Purebasic to thread safe.

2 - Each thread gets its own input, output and working memory area as structure.

3 - Accesses to global memory areas are protected with mutex.

4 - Access to globel maps and lists are protected with mutex and if needed with push and pop the element position is saved and restored.

5 - Do not change the contents of gadgets directly from thread. Works on windows 99% of the time. With Linux and macOS this leads to crashes.

6 - Send messages to the GUI with PostEvent. Any parameter of PostEvent can be used for this. Passing strings to the GUI via PostEvent, allocating the string in thread and deleting the string in the GUI again (AllocateStructure(String)).

7 - Terminate all threads before ending the programme. For example, via an entry (Exit) in the input memory area of the thread. This way all work can be stopped properly. (macOS does not like running threads when exiting the programme).

8 - Do not use the PB functions StopThread and ResumeThread to stop a thread. The thread will be stopped on unknown processing. Alternatively, work with semaphores *.

* Example: Mini Thread Control