TODO: Possible Race Condition in Restore Checkpoint

[Mrockwell2]

icg2/DataTypes/src/MemoryManagement/MemoryManager.cpp

Lines 191 to 192 in aad7ec8

	void MemoryManager::restore_checkpoint( std::istream& in_s) {
	// TODO: We should be holding a lock here

TODO comment turned to issue

[jdeans289]

Thanks for pulling this up here Marcus! We should consider what we want the synch model for the MemoryManger to be. It's a bit ad-hoc in the current Trick, and I think we need to think about it a little harder. We definitely need some synch, since in Trick the memory manager is often accessed via variable server session or data recording threads, so we should work backwards from that and figure out some requirements.

Also - I want to stop using pthreads directly and use the C++ threading library, which gives us things like RAII style locking.

[Mrockwell2]

Just read up on RAII and it looks interesting! If we upgrade to C++ threads, would we also need to upgrade the rest of Trick? I'm pretty sure we'll get some pushback on the change if that's the case.

[jdeans289]

Yeah probably, but it should be contained to Trick internals so hopefully it wouldn't cause issues with our customers.

[Mrockwell2]

Yeah you're probably right