PureBasic Forums - English

Posted: **Thu Jun 09, 2011 6:19 am**

Having just started using threads, it took me a few hours of brain storming to work out what I think is a working thread for programs that require a task to run in the background such as copying files using CopyFile and have another task being a progress window at the same time the main program is running.

The submitted code is a working example of what I did for my MP3 player code to copy files in the background, update a progress window and also be able to continue using the player as usual. I gave it a severe testing the other night while having a couple of wines or three, (well I was feeling a little bit under the weather next morning) and I was impressed at how well it worked, copying hundreds of MP3 files in the background, while I was listening to music and using the other functions of the player at the same time and not have any noticeable glitches.

So to get to the point of this posting, I was wondering if anyone was interested in contributing to it, especially the Gurus of PB coding, pointing out to us dummies of threads, what pit falls to look out for and best syntax practices, without getting too complicated, especially considering tearaway threads could be a potential danger to people's operating systems, and this post also be a helpful tutorial on threads for beginners to PB.

The following code is a working example of what I made up for my MP3 player code to copy files in the background and be a starting point for critical analysis, such as should I use Mutex, or you should put this check in for better security etc.
I think that I have covered most bases to prevent a severe crash, but not absolutely sure if there are any problems that I have not thought of that could crash the program, lock up the computer and in the worst case corrupt the operating system in some way.
I still need to tidy up the way DisableGadget(#btn_Main,0) is called for COPYING COMPLETE message delay and have the button fade back in synchronization. But I have prevented the user entering back into the procedure before it had actually ended with ProgressFlag=0 check.

Code: Select all

Enumeration
  #win_Main
  #win_Progress
EndEnumeration
Enumeration
  #txt_Progress
  #btn_Cancel
  #btn_Main
EndEnumeration

Global Item,Count,ProgressFlag,TaskFlag,Cancel

Procedure ProgressWindow(*Dummy)
  If OpenWindow(#win_Progress,0,0,300,60,"File Copy In Progress",#PB_Window_ScreenCentered)
    TextGadget(#txt_Progress,10,10,280,15,"",#PB_Text_Center)
    ButtonGadget(#btn_Cancel,120,35,60,20,"Cancel")
    Temp=-1 ;Make sure Temp <> Item
    Repeat
      Event=WaitWindowEvent(1)
      If Event=#PB_Event_Gadget And EventGadget()=#btn_Cancel ;Cancel out of the task
        Cancel=1:Break
      EndIf
      If Temp<>Item ;Prevents flickering of the progress text
        Temp=Item
        SetGadgetText(#txt_Progress,"COPYING "+Str(Item)+" OF "+Str(Count)+" FILES")
      EndIf
    Until TaskFlag=0 ;Keeps repeating until TaskWindow() done
    If Cancel=0 ;Skip when Cancel selected =1
      SetGadgetText(#txt_Progress,"COPYING COMPLETE"):Beep(1500,200):Beep(1600,200):Beep(1700,200)
      For Temp=1 To 1500:WaitWindowEvent(1):Next
    Else
      SetGadgetText(#txt_Progress,"COPYING CANCELLED")
      For Temp=1 To 1500:WaitWindowEvent(1):Next
    EndIf
    CloseWindow(#win_Progress)
    ProgressFlag=0 ;Progress thread is done flag
    DisableGadget(#btn_Main,0)
  EndIf
EndProcedure
Procedure TaskWindow(*Dummy)
  For Item=1 To Count
    If Cancel=1:Break:EndIf ;Cancel out of the task
    Delay(1500) ;File Copy or whatever Task here
  Next
  TaskFlag=0 ;Task thread is done flag
  DisableGadget(#btn_Main,0)
EndProcedure

OpenWindow(#win_Main,50,50,200,200,"Thread Play",#PB_Window_SystemMenu|#PB_Window_SizeGadget|#PB_Window_ScreenCentered)
ButtonGadget(#btn_Main,10,10,100,20,"Play Thread")
Repeat
  If EventGadget()=#btn_Main
    If ProgressFlag=0 And TaskFlag=0 ;Make sure both threads are done
      Cancel=0:ProgressFlag=1:TaskFlag=1:Count=5
      DisableGadget(#btn_Main,1)
      ThreadTask=CreateThread(@TaskWindow(),0)
      ThreadProg=CreateThread(@ProgressWindow(),0)
    EndIf
  EndIf
Until WaitWindowEvent()=#PB_Event_CloseWindow And ProgressFlag=0 And TaskFlag=0 ;Make sure both threads are done

End

Some other good sources for reading on the use of threads...
PureBasic Help Documentation
Survival Guide by blueznl
PureBasic - A Beginners Guide by Kale

Posted: **Thu Jun 09, 2011 8:07 am**

Don't cross thread your UI operations. In your threads you call "DisableGadget(#btn_Main,0)" where #btn_Main is a button created in the main thread. Win32 definately isn't designed for cross threaded access to controls. I'm not sure about Linux or Mac, but other OSes I've dealt with are also not thread safe, at the very least you'd have a UI mutex that you'd have to lock before manipulating the UI. Unless PB takes care of this for you, which I don't actually know, but somehow I doubt it.

You are enabling the button in the main window at the end of each thread, but this is different logic from that which is in the main thread, where both threads should be complete before the button is enabled. You should leave the main thread to look after itself rather than mix the logic between the threads. You could send a message back to the main thread to say that it has completed, but thread notification and custom messages in the window event handler loop are limited in PB. It might be simpler just to use a WindowEvent(x) loop and periodically check the state of the threads using IsThread() from the main loop.

It would probably be better to re-organise it so that the main thread only creates one thread, and if that thread then spawns another thread to perform the task or manage the UI then so be it. Actually what I'd probably really do is have all the UI in the main thread, create a progress window for the task and have the task post progress updates be to the main window which then updates the progress window that's associated with that task. But it's not cleanly done in PB.

You've used a global "Count" for the number of files to copy. Why not pass it as the parameter to the thread?

You've used variables "ProgressFlag" and "TaskFlag" to indicate when your threads are running. When you are using a flag across threads like this you need to be sure that the value in memory is seen by both threads. The difficulty is ensuring it has happened in the face of CPU optimisations and caches. And it gets worse in multi-processor/multi-core systems where the caches can be separate. It'll probably work OK most of the time because you're doing such high level operations that it causes memory synchronisation anyway, or there might be a delay until the CPU automatically flushes the cache. It may even work OK when you clear the flag and exit the thread. But it you want to be sure you need to use a proper synchronisation object (like a semaphore) as these will generally cause memory synchronisation. This is a pretty wishy washy explanation, so you're probably better searching for "memory barriers" on Google for in-depth information about the underlying issues.

You might be able to use IsThread() for the above checks too, as the underlying OS mechanism probably checks the state of the thread which will cause a memory synchronisation.

Generally speaking, if you want to be sure it's going to work between threads use a sychronisation object. That's what they're for.

Posted: **Fri Jun 10, 2011 4:41 am**

Wow, thanks for that extensive and valuable feedback tinman

It will take me a couple of days to wrap my head around your suggestions.
Lets see how I go.

Posted: **Fri Jun 10, 2011 7:36 am**

Perhaps this could be useful for some...

http://bluez.home.xs4all.nl/purebasic/p ... #7_threads

Posted: **Sat Jun 11, 2011 5:46 am**

Perhaps this could be useful for some...
http://bluez.home.xs4all.nl/purebasic/p ... #7_threads

Yes it will be bluez, I was a bit lazy and not put a direct link in my first post.
I have re-worked my first posting code, after advice from tinman, still testing and will probably post tomorrow.

Posted: **Sun Jun 12, 2011 5:02 am**

Keeping the advice from tinman in mind I have re-worked the original code from the first post.
First thing I have done is to re-organise the code so as to not cross thread UI operations.
I now have the progress thread spawn the filecopy thread and use lock and unlock mutex functions in the copy thread.
Got rid of the silly flags and use IsThread()=0 for 'is the thread still active' status.
I use one flag because I need to check between two states for an answer: is thread=0 And Flag=1 to disable the button back again, once.
I now pass Count as parameter, was not incorporated originally as I am swapping this code in and out of the MP3 player code, but now I tidy up that code too.
I feel more comfortable with these changes and the code seems to work more smoothly with the #btn_Main status being updated correctly.
Please don't hesitate to let me know if the code need to be altered more, or even submit code example of the way you would handle the same situation.

Code: Select all

Enumeration
  #win_Main
  #win_Progress
EndEnumeration
Enumeration
  #txt_Progress
  #btn_Cancel
  #btn_Main
EndEnumeration

Global Item,Cancel,Mutex

Procedure TaskWindow(Count)
  LockMutex(Mutex)
  For Item=1 To Count
    If Cancel=1:Break:EndIf ;Cancel out of the task
    Delay(Random(2000)+200) ;File Copy or whatever Task here
  Next
  UnlockMutex(Mutex)
EndProcedure
Procedure ProgressWindow(Count)
  If OpenWindow(#win_Progress,0,0,300,60,"File Copy In Progress",#PB_Window_ScreenCentered)
    TextGadget(#txt_Progress,10,10,280,15,"",#PB_Text_Center)
    ButtonGadget(#btn_Cancel,120,35,60,20,"Cancel"):Cancel=0
    Temp=-1 ;Make sure Temp <> Item
    Mutex=CreateMutex()
    ThreadTask=CreateThread(@TaskWindow(),Count)
    If ThreadTask
      Repeat
        Event=WaitWindowEvent(1)
        If Event=#PB_Event_Gadget And EventGadget()=#btn_Cancel ;Cancel out of the task
          Cancel=1:Break
        EndIf
        If Temp<>Item ;Prevents flickering of the progress text
          Temp=Item
          SetGadgetText(#txt_Progress,"COPYING "+Str(Item)+" OF "+Str(Count)+" FILES")
        EndIf
      Until Item=Count
      If Cancel=0
        SetGadgetText(#txt_Progress,"COPYING COMPLETE"):Beep(1500,200):Beep(1600,200):Beep(1700,200)
        For Temp=1 To 1500:WaitWindowEvent(1):Next
      Else
        SetGadgetText(#txt_Progress,"COPYING CANCELLED")
        For Temp=1 To 1500:WaitWindowEvent(1):Next
      EndIf
      Repeat ;Wait until ThreadTask completed
        WaitWindowEvent(1)
      Until IsThread(ThreadTask)=0
    EndIf
    FreeMutex(Mutex)
    CloseWindow(#win_Progress)
  EndIf
EndProcedure

OpenWindow(#win_Main,50,50,200,200,"Thread Play",#PB_Window_SystemMenu|#PB_Window_SizeGadget|#PB_Window_ScreenCentered)
ButtonGadget(#btn_Main,10,10,100,20,"Play Thread")
Repeat
  If EventGadget()=#btn_Main
    If IsThread(ThreadProg)=0
      Count=10
      ThreadProg=CreateThread(@ProgressWindow(),Count)
      If ThreadProg
        Flag=1 ;Used for DisableGadget
        DisableGadget(#btn_Main,1)
      EndIf
    EndIf
  EndIf
  If IsThread(ThreadProg)=0 And Flag=1 ;Only flog DisableGadget once
    SetActiveWindow(#win_Main):DisableGadget(#btn_Main,0):Flag=0
  EndIf
Until WaitWindowEvent()=#PB_Event_CloseWindow And IsThread(ThreadProg)=0

End

Posted: **Wed Jun 15, 2011 10:46 pm**

It's looking better, but I have a few more comments and I have updated your example. I hope you find them useful.

You were not really protecting anything with your use of "Mutex". Generally, you surround accesses to your shared resource that you are protecting with that mutex, with calls to Lock/UnlockMutex(). All the threads accessing that shared resource need to do this, so your TaskWindow() thread was locking the mutex for too long and the ProgressWindow wasn't locking it at all.

You'll probably notice that I've went back on some things, such as using IsThread() everywhere. I realised as I was updating the example that it is not the best thing to do, however rather than using a shared flag to indicate the status of the thread you can use the thread number.

I also found another example here: http://www.purebasic.fr/english/viewtop ... 12&t=45528

Edit: had timer wait logic wrong.

Edit2: Snipped code, see most recent post.

Posted: **Thu Jun 16, 2011 4:12 am**

Thanks for your feedback tinman and the update to my code, this gives me a good standing on the pathway to threading in PB by learning from example.

I thought that my Mutex was probably not doing anything.

That link is handy for me too.

Posted: **Fri Jun 17, 2011 3:47 am**

After spending an hour learning from tinman's example, I have learned quite a lot about how to use threads properly, especially where to lock and unlock the Mutex.
tinman has also showed me much better coding logic than I have used in my examples too and I can use this knowledge to better my programming techniques.
Your help has been much appreciated tinman and I am sure will be helpful for the wider PB community too.

Posted: **Mon Jun 20, 2011 9:47 am**

No problem. Actually I think there are still some mistakes in my code, so I would not use it as the best example. I will correct them tonight and re-post.

Posted: **Mon Jun 20, 2011 7:31 pm**

Here is the fixed code. I basically check the existence of the thread in two steps now, not 1 ("if thread<>0 And IsThread()<>0") otherwise it would always run the else condition code which is just wasteful. I have removed the previous code.

Code: Select all

EnableExplicit

Enumeration
  #win_Main
  #win_Progress
EndEnumeration
Enumeration
  #txt_Progress
  #btn_Cancel
  #btn_Main
EndEnumeration

; Item and Cancel are shared between the TaskWindow() thread
; and ProgressWindow() thread. "Mutex" is used to control access
; to these two
Global Item,Cancel,Mutex

Procedure TaskWindow(count)
  Protected copy_item
  Protected cancelled ; This is a copy of the cancel flag but used locally so that the mutex access isn't a mess of if's and the possibility to miss an unlock
  
  copy_item = 1
  While copy_item<=count And cancelled=0
    
    ; Only lock the mutex while you are using the shared resource
    ; and only keep it for as short a time as possible
    LockMutex(Mutex)
    cancelled = Cancel  ; Get a copy of cancel flag
    Item = copy_item    ; Signal the progress of this thread
    UnlockMutex(Mutex)
    
    ; Only do the work if we have not been cancelled
    If cancelled=0
      Delay(Random(2000)+200) ;File Copy or whatever Task here
      copy_item + 1
    EndIf
  Wend
EndProcedure

Procedure ProgressWindow(count)
  Protected event.i
  Protected task_thread.i
  Protected last_item.i     ; The previous item for thread progress / updating text
  Protected item_progress.i ; Copy of item in progress so we can release the mutex ASAP
  
  last_item = -1
  item_progress = last_item
  
  If OpenWindow(#win_Progress,0,0,300,60,"File Copy In Progress", #PB_Window_ScreenCentered)
    TextGadget(#txt_Progress,10,10,280,15,"",#PB_Text_Center)
    ButtonGadget(#btn_Cancel,120,35,60,20,"Cancel")
    
    ; OK to set shared variables here (Item, Cancel and Mutex) since this is the only thread 
    ; accessing them at this point
    Item = last_item
    Cancel = 0
    Mutex = CreateMutex()
    If Mutex<>0
      task_thread = CreateThread(@TaskWindow(), count)
      While task_thread<>0
        event=WaitWindowEvent(1)
        Select event
          Case 0
            If task_thread<>0
              If IsThread(task_thread)=0
                ; Thread exited
                task_thread = 0
              Else
                ; Thread still running, update progress.
                ; This is where being able to send a custom event from a thread to a window
                ; would be ideal and we would not rely on timeouts for WaitWindowEvent()
                ; Lock mutex while accessing shared resource
                LockMutex(Mutex)
                item_progress = Item
                UnlockMutex(Mutex)
                
                ; Do the actual GUI update outside the mutex
                If last_item<>item_progress ;Prevents flickering of the progress text
                  last_item = item_progress
                  SetGadgetText(#txt_Progress,"COPYING "+Str(item_progress)+" OF "+Str(count)+" FILES")
                EndIf
              EndIf
            EndIf
              
          Case #PB_Event_Gadget
            If EventGadget()=#btn_Cancel ;Cancel out of the task
              ; Lock mutex while accessing shared resource
              LockMutex(Mutex)
              Cancel = 1
              UnlockMutex(Mutex)
              
              DisableGadget(#btn_Cancel, 1)
              SetGadgetText(#txt_Progress,"Cancelling...")
            EndIf
        EndSelect    
      Wend
      
      FreeMutex(Mutex)
    EndIf

    ; The thread has exited by the time we come out of the While loop, so again there
    ; is only one thread accessing the shared resources, so we do not need to use the mutex
    If Cancel=0
      SetGadgetText(#txt_Progress,"COPYING COMPLETE")
    Else
      SetGadgetText(#txt_Progress,"COPYING CANCELLED")
    EndIf
    
    ; Display the end progress for a short time
    AddWindowTimer(#win_Progress, 1234, 1500)
    While WaitWindowEvent()<>#PB_Event_Timer Or EventTimer()<>1234 : Wend
    CloseWindow(#win_Progress)
  EndIf
EndProcedure

Define main_ev.i
Define ThreadProg.i

OpenWindow(#win_Main,50,50,200,200,"Thread Play",#PB_Window_SystemMenu|#PB_Window_SizeGadget|#PB_Window_ScreenCentered)
ButtonGadget(#btn_Main,10,10,100,20,"Play Thread")
Repeat
  ; You should use WaitWindowEvent() before trying EventGadget()
  ; as you could end up checking for a gadget operation when you
  ; have not had a gadget event and if your gadget ID was 0 then
  ; you may perform the action for handling that gadget when you
  ; should not.
  main_ev = WaitWindowEvent(15)
  
  Select main_ev
    ; I have added a timeout to the WaitWindowEvent() because there
    ; is no native cross-platform method in PB for a thread to signal
    ; to another thread that is stuck in a WaitWindowEvent().
    ; When we timeout, we'll check the thread status.
    Case 0
      If ThreadProg<>0
        If IsThread(ThreadProg)=0
          ; I have re-set ThreadProg here rather than purely relying on
          ; IsThread(). I remembered that at an OS level there is no
          ; guarantee that a thread ID will not be re-used. Perhaps someone
          ; from the PB can clarify whether this is required.
          ThreadProg = 0
          
          ; Don't need "Flag" any more, since we are updating the state of ThreadProg
          SetActiveWindow(#win_Main)
          DisableGadget(#btn_Main,0)
        EndIf
      EndIf
      
    Case #PB_Event_Gadget
      If EventGadget()=#btn_Main
        If ThreadProg=0
          ThreadProg = CreateThread(@ProgressWindow(), 10+Random(5))
          If ThreadProg
            DisableGadget(#btn_Main, 1)
          EndIf
        EndIf
      EndIf
      
    ; If you don't want to allow the user to quit until the thread
    ; is complete you should let them know otherwise they might think
    ; your application has crashed. The other strategy is to have some
    ; way to cancel your thread from the main thread.
    Case #PB_Event_CloseWindow
      If ThreadProg<>0
        MessageRequester("Thread Example", "Need to wait for thread to complete before exiting")
      EndIf
  EndSelect
Until main_ev=#PB_Event_CloseWindow And ThreadProg=0

End

Posted: **Wed Jun 22, 2011 3:52 am**

No probs tinman and thanks again.

PureBasic Forums - English

Threads For Dummies

Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies - ThankYou tinman

Re: Threads For Dummies

Re: Threads For Dummies

Re: Threads For Dummies