Threading/Queue Issue about python_dhcp_server HOT 7 CLOSED

hello @waukeerules , seems very interesting on your issue...
Frankly speaking, I am also fresh on this... unitl now what I only know is my following comments , @niccokunzmann please do not hesitate to point out if I make any mistakes... thanks in advance.

Take the first part of your log as an example, when calling self.queue.put((time.time() + seconds, func, args, kw)), priorityqueue use heapq and inside heapq it uses operator < to judge for the priority of current input tuple and previous input tuple.

When comparing the tuple, it first judges the first item of tuple : time.time() , it finds that current tuple and previous tuple has the same value for time.time() (really amazing here ...) , then it compare the second field of tuple , but this time func is a method and can not be compared , thus cause the TypeError: '<' not supported between instances of 'method' and 'method'. (Refer to python code of heapq.py (function _siftdown(heap, startpos, pos):)and here for more information.

So , for conclusion,
Seems that the reason why heapq complains: `TypeError: ' is that there is one tuple in previous priority queue with the same timestamp value of the current input one stored in priorityqueue! sounds really amazing...

Unfortunately, I do not know much about multithread either, but it seems that using current python standard library PriorityQueue and storing timestamp value in queue ( actually in python list) used as priority identifier may be not a good idea for they may be the same value .....

May be we can find another value used as identifier instead of time.time() stored in the priorityqueue. Or using a self implement priorityqueue to avoid such a problem^_^

what do u think @niccokunzmann

from python_dhcp_server.

I found that the errors only occur when multiple DHCP requests are initiated at the same exact time.

from python_dhcp_server.

Hello @waukeerules ,

I modify the dhcp.py , using a simple self-implement PriorityQueue class instead of the official one to avoid the timestamp same value problem. I test it on my bench it works. But for multiple device condition I need your help.

Could you please help me to have a try ? just unzip the file and replace the old one . Hoping for your feedback ! thanks in advance

dhcp.zip

from python_dhcp_server.

Thanks for reporting this issue! I think, I assumed this case never to turn up.

Yes to this: #12 (comment)

To me, it seems this is the culprit:

If we put an additional item in the list, we would also need to change this line:

My solution would be to add a number there which is unique and increased inside a lock, so that there is no multi-threading issue.

_id_lock = threading.Lock()
_id = 0
def get_id():
    global _id
    with _id_lock:
        _id += 1
        return _id

As as an answer to #12 (comment), @greggzj if you create a pull request, we can talk about this and even download the source code of it as a zip file and see the change. I do not know what you did and what the change is and this makes it harder for me, in case this is the accepted solution, to add the change into the source code here.

@greggzj @waukeerules, how would you like to go on from here?

from python_dhcp_server.

@niccokunzmann thanks for feedback and sorry for the inconvenience.

I create a pull request #13 and the newly implemented PriorityQueue class

1. no need to modify this line for the self implement priorityqueue class always return item itself exclude the priority identifier index:
   

   python_dhcp_server/server/dhcp.py

   Line 128 in 32ba283

   t, func, args, kw = p

2. Timestamp value is still used as before.

Seems your solution also works. Which one do u prefer ?

from python_dhcp_server.

Thanks for looking into this issue. I was able to create a simple workaround by changing the queuing method to -first in, first out:

self.queue = queue.Queue()

from python_dhcp_server.

@waukeerules, thanks.... Yep, that is the simplest solution. I used this - the thing is that the packets are re-ordered automatically if it is a queue. So, that is fine and it does not need to be a priority queue.
I am closing this. If anyone finds out that this re-appears, please let me know.

d0d50df

from python_dhcp_server.