search

jimmejardine / qiqqa-open-source

The open-sourced version of the award-winning Qiqqa research management tool for Windows
GNU General Public License v3.0
380 stars 64 forks source link

Race condition somewhere? fetching PDFs using the Sniffer while Qiqqa updates the library (OCR!) in the background randomly locks up Qiqqa mid-activity. #18

Closed GerHobbelt closed 5 years ago

GerHobbelt commented 5 years ago

Given the code for Library.IsBusyAddingPDFs I assume Qiqqa has more multithreading risks like that (a timeout/delay check isn't going to fly at all times and with a large library over here, those 3 seconds isn't going to cut it, resulting in a Qiqqa lock up some of the time)

Random lock up behaviour like this strongly hints towards a race condition somewhere in the code, so I guess we'll have to go through the interlocking/thread interaction inside Qiqqa. 🙉 🙈 😄

Observations

  1. These lockups almost always happen when you download a new/existing(?) PDF from the Sniffer into Qiqqa while both Qiqqa.exe and QiqqaOCR.exe are running.

  2. Dialing down the number of QiqqaOCR.exe instances to 1(ONE) on my i7 dev box seems to reduce the chance of lockup like this happening, but it certainly is still present.

  3. Using Library.IsBusyAddingPDFs in the Infrequent Background Task code seems to have minor negative = worse impact.

  4. My work on SHA-1: 8a1d7660659079939e59be74bf3822ea6311a205 (Fixing https://github.com/jimmejardine/qiqqa-open-source/issues/17) seems to have a strong detrimental = much worse effect as the number of lockup occurrences has increased notably since that change.

    However, since that commit fixed #17 and thus prevents me from short-circuiting to the Windows Task Manager kill -9=end process tree decision, this "much worse" is quite possibly purely psychological as Qiqqa is running much longer now, per session.

GerHobbelt commented 5 years ago

Done as per #33.

locking code has been inspected and fixed in a few places; improved in another spot (SHA-1: a80be7d720e4db3aa514c57a602a4a3794996354)

Note: do not lock(...) on Dictionaries<...> but use a separate, dedicated 'lock' object instead: this helped remove a few additional glitches in the code.

Commits:

Revision: a80be7d720e4db3aa514c57a602a4a3794996354 done work on https://github.com/jimmejardine/qiqqa-open-source/issues/27 and on the lockups of Qiqqa (some critical sections in there were humongous in both code side and run-time duration; now the number of lock-ups due to very slow loading PDFs coming in from the Qiqqa Sniffer should be quite reduced: work related to https://github.com/jimmejardine/qiqqa-open-source/issues/18

Revision: 53f2ca86bc5547888648ab70541999d0c573a981 code cleanup activity (which happened while going through the code for thread safely locks inspection)

Revision: 8b2b3de334f37b0d2def782764e24c177d3a2915 https://github.com/jimmejardine/qiqqa-open-source/issues/18 work :: code review part 2, looking for thread safety locks being applied correctly and completely. Also contains a few lines from work done before related to https://github.com/jimmejardine/qiqqa-open-source/issues/10 et al.

Revision: 5dcda970514c20518d32d7575b747280af8fa24b https://github.com/jimmejardine/qiqqa-open-source/issues/18 work :: code review part 1, looking for thread safety locks being applied correctly and completely: for example, a few places did not follow best practices by using the dissuaded lock(this){...} idiom (https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement)

GerHobbelt commented 5 years ago

Closing and decluttering the issue list so it stays workable for me: fixed in https://github.com/GerHobbelt/qiqqa-open-source mainline=master branch, pending #15 / any maintainer rights/actions.