SBMLTransforms thread safety

[lkeegan]

SBMLTransforms uses a static member variable map mValues for caching component values.

If libsbml is used from multiple threads this can cause corrupted data and/or segfaults since all the threads are using the same map, e.g. if one thread clears the map while another thread is using it (see spatial-model-editor/spatial-model-editor#786)

[fbergmann]

ideally, for a non-changing model, it should suffice to compute the map only once. But this wasnt' the case as the old code always called mapComponentValues and clear later on. Would need to consult with @skeating what else we can do.

[lkeegan]

I think ideally there would be a map for each model, rather than a single libsbml-wide map. (Even without multiple threads and assuming the map is only computed once, if you have two different models open in libsbml wouldn't they currently both use the same mValues map, potentially getting wrong results?)

Some (maybe naive) ideas:
Could mValues be added to the model?
Or if modifying the model is not a good idea, maybe SBMLTransforms could have a separate map for each model, identified by the pointer to the model, e.g. instead of

IdValueMap mValues

something like

std::map<uintptr_t,IdValueMap> map_model_pointer_to_mValues

[fbergmann]

Both approaches would work, i slightly like the latter one more. Since this map would be used / updated / replaces solely by SBMLTransforms. Why would you go with uintptr_t instead of Model*?

[lkeegan]

Not sure why I used a uintptr_t there - Model* would be better!

[fbergmann]

I'm not sure i can get to it this week, but i can have it done by next. Thanks again for letting us know.