Pushing None will mess up the internal state #9

Activity

1. philipbl commented on Nov 30, 2016

   

   philipbl

   on Nov 30, 2016

   Owner

   More actions

   I'm not sure what you mean? This works fine:

       def test_push_none(self):
           self.queue.push(1)
           self.queue.push(None)
           self.assertEqual(self.queue.pop(1), 1)
           self.assertEqual(self.queue.pop(1), None)
   
           self.queue.push(1)
           self.queue.push(None)
           self.queue.push(2)
           self.assertEqual(self.queue.pop(3), [1, None, 2])
2. Kriechi commented on Nov 30, 2016

   

   Kriechi

   on Nov 30, 2016

   ContributorAuthor

   More actions

   Mhm - unfortunately I can't neither remember nor reproduce my initial problem.

   What I do remember:
   I added some push(None) statements to my application, and right after the whole queue went crazy.
   length reported e.g. 24, but when I used peek or pop, it raised an error that it couldn't unpack a header. If I remember correctly, the file position (seek) was at EOF, and then it tried to read 4 bytes, which failed.
3. philipbl commented on Nov 30, 2016

   

   philipbl

   on Nov 30, 2016

   Owner

   More actions

   That's worrying. Was this with the current version or the new version from your PR? Were you using multiple threads?
4. Kriechi commented on Dec 1, 2016

   

   Kriechi

   on Dec 1, 2016

   ContributorAuthor

   More actions

   Yes, current version installed from pypi. I used multiple threads that pushed new items, and a single thread that popped items. I might have restarted the application - so it reloaded / read back in the underlying file.
5. philipbl commented on Dec 1, 2016

   

   philipbl

   on Dec 1, 2016

   Owner

   More actions

   Okay, I was able to reproduce this problem! Here is the code I used:

   https://gist.github.com/philipbl/c3de70dba80bc718b87de289d1f08478

   It is similar to one of the tests, but it pushes None sometimes (line 24). When I remove this line, it works fine. So somehow None is causing issues, like you said. Here is the exception I get:

   Traceback (most recent call last):
     File "thread_test.py", line 38, in <module>
       queue.pop()
     File "/Users/philipbl/Dropbox/Projects/python-persistent-queue/persistent_queue/__init__.py", line 177, in pop
       data, queue_top = self._peek(items, blocking)
     File "/Users/philipbl/Dropbox/Projects/python-persistent-queue/persistent_queue/__init__.py", line 216, in _peek
       data = [read_data() for i in range(total_items)]
     File "/Users/philipbl/Dropbox/Projects/python-persistent-queue/persistent_queue/__init__.py", line 216, in <listcomp>
       data = [read_data() for i in range(total_items)]
     File "/Users/philipbl/Dropbox/Projects/python-persistent-queue/persistent_queue/__init__.py", line 197, in read_data
       length = struct.unpack(LENGTH_STRUCT, self.file.read(4))[0]
   struct.error: unpack requires a bytes object of length 4
   

   I will start looking into it, but I just wanted to document the problem.
6. philipbl commented on Dec 1, 2016

   

   philipbl

   on Dec 1, 2016

   Owner

   More actions

   The problem is in lines 185 and 222. Since data is None, the length is not decremented. There is no way of determining if _peek is returning None because the value it removed from the queue is None or because there is no data in the queue. Instead of returning None when there is no data in the queue, we can do the same thing as Queue and raise an Empty exception.

   Let's finish #7 and then we can fix this problem.
7. philipbl

   modified the milestone: Version 2 on Dec 6, 2016
8. philipbl

   added a commit that references this issue on Dec 14, 2016

   Add test to show problem of putting None

   ...

   67f98c0