glasgowcompbio / vimms

A programmable and modular LC/MS simulator in Python
MIT License
19 stars 6 forks source link

Improve thread-safety #201

Closed joewandy closed 1 year ago

joewandy commented 3 years ago

Some critical codes in ViMMS and ViMMS-fusion don't seem to be thread-safe. For e.g. the TaskManager in ViMMS could potentially be called from multiple threads for MsScanArrive event handlers, or see here.

Ways to confirm if this is a problem:

  1. Create a simple multi-threaded version of the simulator (Simon started some work on this, I think, but it's abandoned).
  2. Decrease the intervals between the MS1 scans to process, and see things break in a subtle way.

Posted in another comment:

In the simulator, there's currently no threading at all, so it's fine.

On IAPI, the codes are certainly not thread-safe.

When running on the real mass spec, all the scan handing codes (including calls to this TaskManager) happen inside the event handler for MsScanArrived event from IAPI. I can't find any documentation on how event handling is called from Pythonnet, but I think we could assume that each handler is called in its own thread.

Therefore it is possible for two scans to arrive at such close intervals that we have both event handling threads running at the same time, reading and writing to the same data structure at the same time, and bad things could happen. In fact, this has been an issue with our codes all along -- even without this TaskManager thing, e.g. see here.

The only reason we never hit any problem with this so far is because only the MS1 scans are processed for computations, while MS2 scans are simply received and stored. The gaps between successive MS1 scans are big enough that we never encountered the problem with thread-safety. But still, it's good to fix this now before it becomes a problem. I've added an issue for this.

joewandy commented 3 years ago

Maybe useful https://stackoverflow.com/questions/55290397/how-to-feed-an-async-generator-with-events-from-python-net

joewandy commented 1 year ago

No resource to fix this.

Things work, but it's quite shaky.