2

I've been using jedie's python ping implementation on Windows. I could be wrong, but when pinging two computers (A and B) from separate threads, ping will return the first ping it receives, regardless of source.

Since it could be an issue with jedie's fork, I reverted to the previous version. (This is the version I'm going to explore below)

I added in a line of code in receive_one_ping: (Line 134 or similar)

recPacket, addr = my_socket.recvfrom(1024) # Existing line
print "dest: {}, recv addr: {}.".format(dest_addr, addr) # New line

This allows us to see the address of the ping we're receiving. (Should be same as the destination IP, right?)

Testing:

ping1() pings a known offline IP (1.2.3.4),
ping2() pings a known online IP (192.168.1.1 - my router)

>>> from ping import do_one

>>> def ping1():
    print "Offline:", do_one("1.2.3.4",1)

>>> ping1()
Offline: None

>>> def ping2():
    print "Online:", do_one("192.168.1.1",1)

>>> ping2()
Online: dest: 192.168.1.1, recv addr: ('192.168.1.1', 0).
0.000403682590942

Now if we do them together: (Using Timer for simplicity)

>>> from threading import Timer
>>> t1 = Timer(1, ping1)
>>> t2 = Timer(1, ping2)
>>> t1.start(); t2.start()
>>> Offline:Online: dest: 192.168.1.1, recv addr: ('192.168.1.1', 0).dest: 1.2.3.4, recv addr: ('192.168.1.1', 0).

0.0004508952953870.000423517514093

It's a little mangled (due to print not working nicely with threading), so here it is a bit clearer:

>>> Online: dest: 192.168.1.1, recv addr: ('192.168.1.1', 0).
Offline:dest: 1.2.3.4, recv addr: ('192.168.1.1', 0). # this is the issue - I assume dest should be the same as recv address?

0.000450895295387
0.000423517514093

My questions:

  1. Can anyone recreate this?

  2. Should ping be behaving like this? I assume not.

  3. Is there an existing ICMP ping for python that will not have this behaviour?
    Alternatively, can you think of an easy fix - ie polling receive_one_ping until our destination matches our receive address?

Edit: I've created an issue on the python-ping github page

Édouard Lopez
  • 40,270
  • 28
  • 126
  • 178
Alex L
  • 8,748
  • 5
  • 49
  • 75

1 Answers1

5

This is happening because of the nature of ICMP. ICMP has no concept of ports, so all ICMP messages are received by all listeners.

The usual way to disambiguate is to set a unique identifier in the ICMP ECHO REQUEST payload, and look for it in the response. This code appears to do that, but it uses the current process id to compose the ID. Since this is multithreaded code, they will share a process id and all listeners in the current process will think all ECHO REPLYs are ones they themselves sent!

You need to change the ID variable in do_one() so that it is per-thread unique. You will need to change this line in do_one():

my_ID = os.getpid() & 0xFFFF

Possibly this will work as an alternative, but ideally you should use a real 16-bit hashing function:

# add to module header
try:
    from thread import get_ident
except ImportError:
    try:
        from _thread import get_ident
    except ImportError:
        def get_ident():
            return 0

# now in do_one() body:
my_ID = (get_ident() ^ os.getpid()) & 0xFFFF

I don't know if this module has any other thread issues, but it seems to be ok from a cursory examination.

Using the jedie implementation, you would make a similar change for the Ping() own_id constructor argument. You can either pass in an id you know to be unique (like above) and manage Ping() objects yourself, or you can change this line (110) in the constructor:

self.own_id = os.getpid() & 0xFFFF

Also see this question and answer and answer comment thread for more info.

Community
  • 1
  • 1
Francis Avila
  • 31,233
  • 6
  • 58
  • 96
  • 1
    Excellent, thank you. I think that we only see one reply for each thread because `receive_one_ping` returns when `packetID == ID`, and since they're currently always the same ID, it'll always return the first reply. I think I'll get the thread to pass a unique ID as a parameter to `do_one` - I'll try `threading.current_thread().ident` – Alex L Jan 17 '12 at 03:38
  • Ah, that's exactly it. OK, so this ident issue is likely the only problem. If you prefer the jedie implementation, you can probably use it without modification as long as you pass your custom id to the `Ping()` constructor. – Francis Avila Jan 17 '12 at 03:40
  • `threading.current_thread().ident` works (I believe thread is deprecated): Online: id: 2688 ip: 192.168.1.1. dest: 192.168.1.1, recv addr: ('192.168.1.1', 0). 0.000427707990752 Offline: id: 5492 ip: 1.2.3.4 None. Thanks! – Alex L Jan 17 '12 at 03:57
  • Note that your id needs to be machine-unique. For a single threaded program, the process-id is perfect. For a multithreaded program, however, the thread id from `threading` will only be guaranteed unique within the current process. That's why I XORed with the process id. – Francis Avila Jan 17 '12 at 03:59
  • +1 for explaining the 'ICMP from multiple threads' issue where one thread receives a gnip from a ping issued by another thread ,(and explaining the solution as well). – Martin James Jan 17 '12 at 10:44
  • Francis, you are my hero. I spent two days with similar issue, and with your explanation everything resolved in 5 minutes. ident of current thread work perfect. Thanks! – Ihor Pomaranskyy Mar 05 '12 at 13:26