[Tutor] threading issues

[Kent Johnson]

Chris Hallman wrote:

> I was finally able to get my script to not show duplicate PINGs. I also 
> realized that my script was not PINGing all the hosts in the input file. 

Congratulations on getting it to work! See below for a few notes.

> Here is my latest version:
> 
> 
> import os, re, string, sys, threading, time
> from threading import Thread
> from time import strftime
> 
> ms = re.compile("Reply")

You don't need a regular expression for this, you could use the simpler
  if "Reply" in pingas

> rpath = (r"c:\utils\network_ping_devices.txt")
> 
> if os.path.exists(r"c:\utils\network_ping_again.txt"):
>     rpath = (r"c:\utils\network_ping_again.txt")
> wpath = (r"c:\logs\network_ping.out")
> tpath =  (r"c:\temp\ping.txt")
> if os.path.exists(tpath):
>     os.remove(tpath)
> temp = open(tpath, "w")
> output = open(wpath, "a")
> output.write("\n" + "Network PING test started -" + strftime(" %H:%M:%S 
> %x") + "\n")
> output.flush()
> 
> class PingIT(threading.Thread):
>     def __init__(self,rtr):
>         Thread.__init__(self)
>         self.rtr = rtr
> 
>     def run(self):
>         pingaf = os.popen('ping -n 1 -w 3000 ' + self.rtr)
>         pingas = string.join(pingaf.readlines())
>         if ms.search(pingas):
> #            print (re.sub('\n','',self.rtr)) + " responded."    #for 
> debugging

>             return
>         else:
>             pingaf = os.popen('ping -n 1 -w 3000 ' + self.rtr)
>             pingas = string.join(pingaf.readlines())
>             if ms.search(pingas):
> #                print (re.sub('\n','',self.rtr)) + " responded."    # 
> for debugging

You don't need a regex for simple string replacements, either - you can use
  self.rtr.replace('\n', '')

In this case the \n is at the end of the string - it is included in the lines you read from the file - so you could also use self.rtr.strip().

But even better would be to strip the '\n' from rtr when you read it from the file below, then you wouldn't have to deal with it everywhere else.

>                 return
>             else:
>                 temp.write(re.sub('\n','',self.rtr) + " did not respond.\n")

Instead of writing the results to a file you could accumulate them in a list and sort the list. Make a global
  failed = []
before you start any threads, then use
  failed.append(re.sub('\n','',self.rtr) + " did not respond.")

> 
> pinglist = []
> for rtr in file(rpath):

Here you should say
  rtr = rtr.strip()
to get rid of that annoying trailing newline

>     current = PingIT(rtr)

Yes, that's exactly right - you pass the value of rtr as a parameter to the thread so it doesn't matter that the global value changes.

>     pinglist.append(current)
>     current.start()
> 
> for pingle in pinglist:
>     pingle.join()
> 
> temp.close()
> temp = open(tpath, "r")
> lines = []
> for line in temp:
>     lines.append(line.rstrip())
>     lines.sort()

The sort should be outside the loop, you only have to sort once. If you use the failed list as I suggest above, here you just need
  failed.sort()
then
  for line in failed:

> for line in lines:
>     print >>output, line
> 
> output.write("Network PING test completed -" + strftime(" %H:%M:%S %x") 
> + "\n")
> output.flush()
> output.close()
> 
> 
> All the hosts in the input file are sorted alphabetically but my results 
> were output out of order?! (possibly due to latency in PING responses or 
> some latency in the thread joining; not sure of either). I had to do the 
> crude temp output file so that I could sort the results, but it works!! 

Using threads introduces some indeterminance into the program. You can't be sure that the threads will run in the order that you start them, or that they will run to completion without interruption. And since the ping itself can take different amounts of time that affects the order in which they complete also.

Kent
-- 
http://www.kentsjohnson.com