Thread-Safe and RAII conformant getData() and modifyData() pool calls #74

New Issue

muellerr · 2020-05-11T12:09:44+02:00

muellerr commented

2020-05-11 12:09:44 +02:00

As discussed in #68 and #60 ,
I implemented a thread-safe and RAII conformant access wrapper for the modifyData() and getData() calls. Using this wrapper/accessor allows to reduce the number of output/input variables to a minimum: The call takes the storeId and returns a std::pair consisting of a returnvalue and a StoreAccessor class instance (move semantics) which can be used to access and modify the locked datapool.

The current test code uses the new DHB interface so it doesn't make a lot of sense to make a pull request right now, I can't check whether it compiles now without changing the test code back or writing unit tests, I stil requested a WIP pull request to allow reviewers to inspect a first version.

An alternative constructor could be written which takes a reference to a previously created StoreAccessor() and assigns it. Good thing about the ctor which simply takes a storeId is that it prevents using an uninitialized StoreAccessor instance.

When the accessor exits the scope, itis destroyed, unlocks the mutex und deletes the data. If deletion is not desired, release() can be used to prevent deletion.
Manual locking and unlocking is possible with lock(...) and unlock() .

Some other adaptions in other parts of the code:

I can't use exceptions so I used an internal state the track failed mutex locks.
I had to adapt the StorageManagerIF*, I need an explicit function to lock the pool without a mutex lock, otherwise a double lock is attempted.

I also adapted the mutexHelper to have an internal state, because I could not use exceptions in the constructor to track failed locks.

The accessor it also use of modern C++ features like move semantics and smart pointers.

As discussed in #68 and #60 , I implemented a thread-safe and RAII conformant access wrapper for the modifyData() and getData() calls. Using this wrapper/accessor allows to reduce the number of output/input variables to a minimum: The call takes the storeId and returns a std::pair consisting of a returnvalue and a StoreAccessor class instance (move semantics) which can be used to access and modify the locked datapool. The current test code uses the new DHB interface so it doesn't make a lot of sense to make a pull request right now, I can't check whether it compiles now without changing the test code back or writing unit tests, I stil requested a WIP pull request to allow reviewers to inspect a first version. An alternative constructor could be written which takes a reference to a previously created StoreAccessor() and assigns it. Good thing about the ctor which simply takes a storeId is that it prevents using an uninitialized StoreAccessor instance. When the accessor exits the scope, itis destroyed, unlocks the mutex und deletes the data. If deletion is not desired, `release()` can be used to prevent deletion. Manual locking and unlocking is possible with `lock(...)` and `unlock()` . Some other adaptions in other parts of the code: I can't use exceptions so I used an internal state the track failed mutex locks. I had to adapt the StorageManagerIF*, I need an explicit function to lock the pool without a mutex lock, otherwise a double lock is attempted. I also adapted the mutexHelper to have an internal state, because I could not use exceptions in the constructor to track failed locks. The accessor it also use of modern C++ features like move semantics and smart pointers.

muellerr referenced this issue

2020-05-11 12:48:55 +02:00

WIP: added files for inspection #76

muellerr added the

feature

label 2020-05-11 12:50:17 +02:00

muellerr commented

2020-05-12 16:44:41 +02:00

Accessor was simplified and only handles storage deletion now.

Two new calls for modifyData() and getData() could be used:

Fist call takes a store ID and returns a pair of a returnvalue and a (Const)Storage Acessor instance
Second call takes a store ID and a reference to a storage helper instance (Const)StorageAccessor

Maybe something for the future:
I suggest a lockPool() and unlockPool() call for the PoolManager, in case the developer still wants thread-safe access (however, this propably would require a special PoolManagerIF which just requires implementing those functions)

Accessor was simplified and only handles storage deletion now. Two new calls for modifyData() and getData() could be used: - Fist call takes a store ID and returns a pair of a returnvalue and a (Const)Storage Acessor instance - Second call takes a store ID and a reference to a storage helper instance (Const)StorageAccessor Maybe something for the future: I suggest a lockPool() and unlockPool() call for the PoolManager, in case the developer still wants thread-safe access (however, this propably would require a special PoolManagerIF which just requires implementing those functions)

muellerr referenced a pull request that will close this issue

2020-07-06 12:58:41 +02:00

New StoreAccessors and separation of implementation for storagemanager classes #92

gaisser closed this issue

2020-09-29 14:57:07 +02:00

Sign in to join this conversation.