[Tutor] Threads threads threads gaaahgh (foaming at the mouth)

[Allan Crooks]

After having a quick look at the threading library, I started adjusting your code in quite a few ways.

First of all, here's the code you submitted:
> import threading
> from random import randint
> 
> 
> class Soup:
> 
>  def __init__(self):
>   self.next = 0
>   self.isFull = 0
>   self.isEmpty = 1
>   self.buff = ""
> 
>  def eat(self):
> 
>   while self.isEmpty:
>    try : wait()  #########
>    except: pass
> 
>   self.next = self.next - 1
> 
>   if not self.next:
>    self.isEmpty = 1
> 
>   self.isFull = 0
>   notify()     ########=======> how to do this?
> 
>   return self.buff[next]
> 
> 
>  def add(self,c):
>   while self.isFull:
>    try: wait()    ########===========> this as well
>    except: pass
>   self.buff = self.buff[:self.next]+c+self.buff[self.next:]
>   self.next += 1
> 
>   if self.next == len(self.buff):
>     self.isFull= 1
>   self.isEmpty = 0
>   notify()     #####
> 
> class Consumer:
> 
> 
>  def __init__(self,soup):
>   self.soup = soup
> 
>  def run(self):
> 
>   for i in range(10) :
>    c = self.soup.eat();
>    print "Ate a letter "+c
> 
>    try:
>     sleep(randint(1,3))
> 
>    except:
>     pass
> 
> class Producer:
> 
> 
>  def __init__(self,soup):
>   self.soup = soup
>   self.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> 
>  def run(self):
> 
>   for i in range(10):
>    c = self.alphabet[randint(1,27)]
>    self.soup.add(c);
>    print "Added "+c+" to the soup"
>    try: sleep(randint(1,3))
>    except: pass
> 
> 
> def main():
> 
>  s = Soup()
> 
>  p1 = Producer(s)
>  s1 = Consumer(s)
> 
>  t1 = threading.Thread(target=p1.run)
>  t2 = threading.Thread(target=s1.run)
> 
>  t1.start()
>  t2.start()
> 
>  try :
> 
>   t1.join()
>   t2.join()
> 
>  except:
> 
>   print "The producer or consumer thread's join() was interrupted..."
> 
> 
> 
> if __name__ == '__main__':
>  main()
> 


Here's the modified code. The comments (#01, #02 etc.) are used as markers, so I can illustrate how your code has been modified.

import threading
from time import sleep #01
from random import randrange #02

class Soup:

 def __init__(self):
  self.next = 0
  self.isFull = 0
  self.isEmpty = 1
  self.buff = ""
  self.lock = threading.Condition() #03

 def eat(self):
  self.lock.acquire() #04
  while self.isEmpty:
    self.lock.wait()  #05

  self.next = self.next - 1

  if not self.next:
   self.isEmpty = 1

  self.isFull = 0
  self.lock.notify() #06

  self.lock.release() #07
  return self.buff[self.next] #08


 def add(self,c):
  self.lock.acquire() #09
  while self.isFull:
    self.lock.wait() #10
  self.buff = self.buff[:self.next]+c+self.buff[self.next:]
  self.next += 1

  if self.next == len(self.buff):
    self.isFull = 1
  self.isEmpty = 0
  self.lock.notify() #11
  self.lock.release() #12

class Consumer:


 def __init__(self,soup):
  self.soup = soup

 def run(self):

  for i in range(10) :
   c = self.soup.eat();
   print "Ate a letter", c #13
   sleep(randrange(1,5)) #14

class Producer:


 def __init__(self,soup):
  self.soup = soup
  self.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"

 def run(self):

  for i in range(10):
   c = self.alphabet[randrange(26)] #15
   self.soup.add(c);
   print "Added", c, "to the soup" #16
   sleep(randrange(1,3)) #17


def main():

 s = Soup()

 p1 = Producer(s)
 s1 = Consumer(s)

 t1 = threading.Thread(target=p1.run)
 t2 = threading.Thread(target=s1.run)

 t1.start()
 t2.start()

 try :

  t1.join()
  t2.join()

 except:

  print "The producer or consumer thread's join() was interrupted..."



if __name__ == '__main__':
 main()



--------------

OK, so here's an explanation of the changes.

#01: You forgot to import sleep from the time module.
#02: We're using randrange instead of randint, because randrange(x,y) will return a number between x (inclusive) and y (exclusive). randint returns a number between x (inclusive) and y (inclusive). It doesn't make much of a difference, it's just what I've chosen to use.
#03: We're using "condition" objects to deal with locking and unlocking. We need to use these so we can do notify() and wait().
#04: Here we've "acquired" the lock. We need to do this before we can do "wait" or "notify"
#05: Now we're using the condition object and waiting on it.
#06: This is the notify that you wanted to put in the code, and we're just invoking the method on the condition object.
#07: We must remember to release the lock once we have finished.
#08: Your code said "return self.buff[next]". What we meant was: "return self.buff[self.next]".
#09: Same as #04.
#10: Same as #05.
#11: Same as #06.
#12: Same as #07.
#13: Doesn't do anything different, but it looks nicer than "Ate a letter "+c. :)
#14: We're just using the randrange method. You may have noticed I adjusted the parameters for randrange, that's because I was just testing with different parameters.
#15: The initial version was more complicated. First of all, the code assumed that the string indexed from 1 (it doesn't). Secondly, changing 1, 27 to 0, 26 still didn't work, because randint may pick a number from 0 to 26, including 26 itself (which is an invalid index). So we can change it either to randint(0,25) or randrange(0,26). I've chosen the randrange option.
#16: Same as #13, it looks nicer. :)
#17: Just using randrange again.

Now having changed that code, I get this:

>>> soup.main()
Ate a letter Y
Added Y to the soup
Ate a letter R
Added R to the soup
Added M to the soup
Added X to the soup
Ate a letter X
Added A to the soup
Added F to the soup
Ate a letter F
Added S to the soup
Added A to the soup
Added G to the soup
Ate a letter G
Added P to the soup
Ate a letter P
Ate a letter A
Ate a letter S
Ate a letter A
Ate a letter M

The code takes several seconds to execute. It seems to work properly.

There are a few more things I feel I should point out about your code for the sake of improvements:
  1) You don't need to use a "next" variable, you can simply use the len function.
  2) I think it'd be easier and nicer to write if you used a list object for a buffer instead of a string.
       That way, the line in the eat function:
          return self.buff[self.next]
       would become:
          return self.buff.pop()

        And the line in the add function:
          self.buff = self.buff[:self.next]+c+self.buff[self.next:]
        would become:
          self.buff.append(c)
   3) Your code had too many "excepts" in it. That in itself is not a bad thing, but you were using "except:" rather than catching any specific errors (e.g. "except: (AttributeError, IndexError), e"). That's definitely something you should aim to change in your programs in the future. For example, your code in the run method of Consumer had these lines in it:

>    try:
>     sleep(randint(1,3))
> 
>    except:
>     pass

Invoking the sleep function doesn't actually raise any exception, but in the code supplied, sleep was undefined (because it wasn't imported). But because you use caught all types of exceptions (including NameErrors), it was harder to track down why no thread seemed to be waiting. If you want to catch a particular exception, you should explicitly state their types, otherwise errors which you would want to get through will not.

HTH,
Allan.