voipmonitor / sniffer

VoIPmonitor sniffer sources
228 stars 107 forks source link

Fix for multiple events being skipped during an inotify read #10

Closed rgagnon24 closed 9 years ago

rgagnon24 commented 9 years ago

Sometime after 10.0.8, the inotify events were not all processed. Only the first event in the queue during a read() operation would be used, and anything after that first one would be lost. This causes files to be skipped in the scandir if a lot of files show up there at once.

Patch changes listFilesDir() to return a <queue> (which operates as a FIFO) instead of a <vector>, so that the queue can continue to be used as a stack to put work on, and for the reading system to use to find work.

Code that uses inotify system to watch the "scanpcapdir" adds all of the events it reads to the end of the queue, and we simply pop work off the end as needed.

Now, the queue is initialized with anything pre-existing in the directory, code then drops into the "run forever" loop, and processes anything in the queue before checking if inotify buffer has anything to do.

Every time the queue empties out, we go into the blocking read() call, grab any events waiting there, and stick them onto the queue. The process then repeats, and no files are inadvertently skipped.

Scenario: Imagine 3 inotify events are read at one time during the read() call:

Under the old code, the first event is read off the queue, and the file is processed. The remaining 2 events are ignored, possibly leaving calls in memory, and obviously not creating a complete SIP trace of a call, if more messages for the calls in the first file are contained in the next 2 files.

With this patch, all three files are pushed into the queue. The first one is popped out of the queue and processed. When the loop comes around again, the queue is not empty, and will pop the 2nd file off and process it, and likewise for the 3rd file.

Now that all 3 files are processed, the queue is empty, and we land back at the read() call to get more work.

voipmonitor commented 9 years ago

Hi thank you very much for the catch! We have applied your patch to the develop branch with one minor change, the read(...,1024) would be maybe not enough so it was extended to 4096 although one inotify event seems to be 32b so 1024 would handle 32 events without overlap we feel more secure to extend the buffer to 4092. proper solution should be probably try to read more data in case the event overlaps.