Closed GerHobbelt closed 4 years ago
The crashes in Qiqqa (as reported in this issue and others, e.g. #98) are at least in part due to thread-unsafe coding of the PDFDocument
class, while references to PDFDocument instances are poassed to multiple threads and async event handlers.
After some contemplation, it was decided to make PDFDocument
threadsafe by introducing mandatory lock()
ing inside every public interface.
To ensure serialization and regular within-thread larger code thunks working on a PDFDocument instance is still performant and backwards compatible, the original PDFDocument class was moved into a "ThreadUnsafe" namespace and renamed to PDFDocument_ThreadUnsafe
so every bit that uses this unsafe codebase directly will be utterly aware that it is doing so. The new PDFDocument implementation is a wrapper (facade pattern) around this old unsafe code, while the new PDFDocument API is made threadsafe by properly applying lock-ing everywhere.
Inks, Annotations and Highlights still pass their lists by reference to callers instead of (deep) copying, hence these bits are still thread-unsafe while the code in PDFDocument might look safe there as there's a basic lock() provided, but as anyone savvy in multiprocess coding knows, that cute critical section is not enough.
This is a TODO that should be addressed in a separate issue.
Closing and decluttering the issue list so it stays workable for me: fixed in https://github.com/GerHobbelt/qiqqa-open-source mainline=master branch, i.e. v82pre3 experimental release and beyond.
Running the app at a machine load of sustained 100% (thanks to Qiqqa OCR background processes).
The crash occurs when you edit the BibTeX of a document in the Sniffer, but CPU load is so high that the responsiveness is reduced, so by the time Qiqqa has queued the document for storage in a background task (passing an implicit object reference of
pdf_document
) your next keypress == user edit comes in and it can happen that the Write action coincides such that .NET internals will cause the JSON serializer to barf because "the list has changed" -- which it did!Either we throw critical sections all over the place or we change the application such that it passes a COPY of the
pdf_document
across thread boundaries.Re that lingering user edit due to slow responsiveness: that SHOULD be dealt with already by yet another Save instruction being queued in that same thread-crossing queue as before.
Hence at least SaveToMetaData() has to be made thread-safe by copy-instancing the pdf document and the metadata therein, i.e. a (semi)DEEP COPY is called for AFAICT.