Strange output for threaded subprocesses

Question

thanks for taking the time to look at this, any help with this would be most appreciated!

I am trying to get some network stats by pinging a list of IP address' called ips. The problem i'm having however is that my output is a list containing several 'None's'. Before implementing threads to run the sub-process commands, the output shown below was a series of numbers. If anybody can take a look at my source code and shed some light on the issue i'd be very grateful!

Thank you in advance!

import subprocess
import re
import threading
from multiprocessing import Pool, Lock
from multiprocessing.dummy import Pool as ThreadPool



def get_ips():

    # Fill empty list with IP address
    ips = []
    with open('C:\Python26\ARPips.prn','r')as f:
        for line in f:
            line = line[:-1]
            if line != "end":
                ips.append(line)
        return ips

def ping(pingArgs):


    lock = Lock()
    lock.acquire()

    # Ping with "pingArgs" as the arguments
    ping = subprocess.Popen(pingArgs,
        stdout = subprocess.PIPE,
        stderr = subprocess.PIPE,
        shell=True)

    # Get and parse output
    out = ping.communicate()
    out = ''.join((out))

    lost = re.findall(r"Lost = (\d+)", out)
    minimum = re.findall(r"Minimum = (\d+)", out)
    maximum = re.findall(r"Maximum = (\d+)", out)
    avg = re.findall(r"Average = (\d+)", out)
    no =  re.findall(r"Sent = (\d+)", out)

    # Change output to integers
    lost = [int(x) for x in lost]
    minimum = [int(x) for x in minimum]
    maximum = [int(x) for x in maximum]
    avg = [int(x) for x in avg]
    no = [int(x) for x in no]



    print "%s \t \t %s \t \t%s \t \t %s \t \t%s" % (no, lost, maximum, minimum, avg)
    lock.release()

def main():


    # grab IP address list
    ips = get_ips()

    # Declare global variables
    global position, newIP, pingArgs

    position = 0
    newIP = ips[position]
    position += 1
    pingArgs = ["ping", "-n", "1", "-l", "1", "-w", "100", newIP]


    # Header for output
    print "Packets \t loss(%) \t Max(ms) \t Min(ms) \t Average(ms)"

    # Instantiate Pool objects, and set size of pool
    pool = Pool(processes=12)


    #Ping ips in own threads and return the results
    result = pool.map(ping, ips)


    # Close the pool and wait for work to finish
    pool.close()
    pool.join()

    # print the results
    print result

if __name__ == '__main__':
    main()
    print get_ips()

The output is shown here below:

Packets      loss(%)     Max(ms)     Min(ms)     Average(ms)
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[]       []         []       []         []
[None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None]
['10.10.10.1', '10.10.10.41', '10.10.10.42', '10.10.10.43', '10.10.10.49', '10.10.10.51', '10.10.10.61', '10.10.10.71', '10.10.10.91', '10.10.10.92', '10.10.10.201', '10.10.10.205', '10.10.10.208', '10.10.10.209', '10.10.10.213', '10.10.10.214']

Process finished with exit code 0

Answer 1

The problem i'm having however is that my output is a list containing several 'None's'

The Nones are produced by print result where result = pool.map(ping, ips) because ping() function doesn't return anything (it means it returns None in Python).

Before implementing threads to run the sub-process commands, the output shown below was a series of numbers.

You are passing ip addresses to Popen() instead of the full ping command to run.

You are defining global pingArgs that is not used anywhere (local pingArgs in ping() overshadows it). You could replace pool.map by builtin map call that creates the result list in the same process and in the same thread, to see that the result is the same (regexes do not match) and the error is not related to threads/processes.

Local lock = Lock() is useless.

Here's the code where the above issues are fixed:

#!/usr/bin/env python
import re
from multiprocessing.dummy import Pool # use threads
from subprocess import Popen, PIPE

def get_ips(filename):
    ips = []
    with open(filename) as f:
        for line in f:
            line = line.strip()
            if line and line != "end":
                ips.append(line)
    return ips

def ping(ip):
    cmd = ["ping", "-n", "1", "-l", "1", "-w", "100", ip]
    ping = Popen(cmd, stdout=PIPE, stderr=PIPE)

    # Get and parse output
    output, err = ping.communicate()
    out = ''.join([output, err])

    lost = re.findall(r"Lost = (\d+)", out)
    minimum = re.findall(r"Minimum = (\d+)", out)
    maximum = re.findall(r"Maximum = (\d+)", out)
    avg = re.findall(r"Average = (\d+)", out)
    no =  re.findall(r"Sent = (\d+)", out)

    # Change output to integers
    lost = [int(x) for x in lost]
    minimum = [int(x) for x in minimum]
    maximum = [int(x) for x in maximum]
    avg = [int(x) for x in avg]
    no = [int(x) for x in no]

    return "%s \t \t %s \t \t%s \t \t %s \t \t%s" % (
        no, lost, maximum, minimum, avg)

def main():
    # grab IP address list
    ips = get_ips(r'C:\Python26\ARPips.prn')

    # Header for output
    print "Packets \t loss(%) \t Max(ms) \t Min(ms) \t Average(ms)"

    # Instantiate Pool objects, and set size of pool
    pool = Pool(processes=12)

    #Ping ips in own threads and return the results
    results = pool.map(ping, ips)

    # Close the pool and wait for work to finish
    pool.close()
    pool.join()

    # print the results
    print "\n".join(results)

if __name__ == '__main__':
    main()

Answer 2

This is what got it working for me (unix) system

import subprocess
import re
from multiprocessing import Pool, Lock

lock = Lock()
pingArgs = "ping -n -l 1 -w 100 "

def get_ips():
    # Fill empty list with IP address
    ips = []
    with open('C:\\Python26\\ARPips.prn', 'r')as f:
        for line in f:
            line = line[:-1]
            if line != "end":
                ips.append(line)
        return ips

def ping(ip):

    global lock
    pingCommand = pingArgs + ip
    lock.acquire()
    # Ping with "pingArgs" as the arguments
    ping = subprocess.Popen(pingCommand,
        stdout = subprocess.PIPE,
        stderr = subprocess.PIPE,
        shell=True)

    # Get and parse output
    out = ping.communicate()
    out = ''.join((out))

    lost = re.findall(r"Lost = (\d+)", out)
    minimum = re.findall(r"Minimum = (\d+)", out)
    maximum = re.findall(r"Maximum = (\d+)", out)
    avg = re.findall(r"Average = (\d+)", out)
    no =  re.findall(r"Sent = (\d+)", out)

    # Change output to integers
    lost = [int(x) for x in lost]
    minimum = [int(x) for x in minimum]
    maximum = [int(x) for x in maximum]
    avg = [int(x) for x in avg]
    no = [int(x) for x in no]

    lock.release()
    return "%s \t \t %s \t \t%s \t \t %s \t \t%s" % (no, lost, maximum, minimum, avg)

def main():

    # grab IP address list
    ips = get_ips()

    # Header for output
    print("Packets \t loss(%) \t Max(ms) \t Min(ms) \t Average(ms)")

    # Instantiate Pool objects, and set size of pool
    pool = Pool(processes=12)

    #Ping ips in own threads and return the results
    results = pool.map(ping, ips)

    # Close the pool and wait for work to finish

    pool.close()
    pool.join()
    # print the results
    for result in results:
        print(result)

if __name__ == '__main__':
    main()
    print(get_ips())

The issues I was experiencing with the code were incorrect ping arguments (-n didn't take any additional parameters), lock should be global, when shell=True it didn't take a list, only a string, and ping(pingArgs) could overwrite the global argument, whereas you want the pool to send only the ip to the worker and then the worker should add it as the final argument.

also I noticed you didn't escape your path to the file on dist, I didn't use/test that part but included it escaped properly as well for reference.

Edit: Also changed it so each function would simply return the value to be printed and print all of them when completed. Instead of having them being printed inside the function as well as the results of the functions without returns (the list of Nones).