uniba-swt / libbidib

A library for communication with a BiDiB (www.bidib.org) system using a serial connection.
GNU General Public License v3.0
10 stars 4 forks source link

WIP: Merge mutex to read-write-lock rewrite (branch read-write-locking) into Master #15

Closed BLuedtke closed 2 years ago

BLuedtke commented 2 years ago

After analysis with the data-race detection tools Helgrind and DRD (both part of Valgrind), the existing mutex setup for preventing concurrent access to shared variables was found to be generally well written, but not devoid of data races.

With the existing mutex setup, protection of reads of shared variables could be done in two ways: Lock the mutex, or don't. Locking the mutex protects against concurrent/conflicting writes, but prevents unproblematic concurrent reads. Not locking at all has the inverse (dis-)advantages: Allows concurrent reads, but does not protect against concurrent writes. Both variants existed for some of the highlevel methods (e.g. bidib_get_board_features in _bidib_highlevelgetter locks no mutex but iterates over _bidibboards, whilst bidib_get_board_id in the same file does lock the mutex to read from _bidibboards.)

With the changes in the read-write-locking branch, the access to the shared variables bidib_boards, bidib_trains, bidib_track_state is now protected with pthread-read-write-locks instead of mutexes. In particular, the following mutexes were replaced:

pthread_mutex_t bidib_state_trains_mutex --> pthread_rwlock_t bidib_state_trains_rwlock
pthread_mutex_t bidib_state_track_mutex --> pthread_rwlock_t bidib_state_track_rwlock
pthread_mutex_t bidib_state_boards_mutex --> pthread_rwlock_t bidib_state_boards_rwlock

These three mutexes in particular were replaced as the chance for concurrent read access is far higher than for other shared objects guarded with mutexes in this library. They frequently appear in the functions available over the highlevel API, which can be called concurrently.

Where previously a function accessing e.g. the bidid_boards array acquired the mutex bidib_state_boards_mutex and only read from the array, the function now acquires a read-lock on bidib_state_boards_rwlock and reads. A function that modifies the array now acquires a write-lock on bidib_state_boards_rwlock instead.
Multiple threads can hold a read-lock at the same time. Only one thread can hold a write-lock at any one time. Whilst a write-lock is being held by a thread, no other thread can hold a read-lock. This ensures that concurrent reads are possible, but write/read, read/write, and write/write conflicts are protected against.

The advantages of read-write locks are:

The disadvantages of read-write locks are:

Inline with the lock changes, const has been added to pointer variables and arguments/parameters where sensible. This shall improve the readability and self-documentation of functions, as it is clear from the parameters whether a fct will modify a pointer argument or not.

In the process of implementing the changes outlines above, some experimental changes were made to the physical test suite for the SWTbahn-Full. Furthermore, some logging calls were temporarily disabled as Hellgrind would confuse the variadic argument replacement with writes to shared objects. These are the main reasons for why this PR is explicitly still Work-in-Progress. In the course of the next commits, the experimental changes will be either reverted or adjusted to fit the conventions in the master branch. Moreover, more tests shall be performed. So far, the changes were tested with the physical test suite for the SWTbahn-Full, but not thoroughly with the unit tests. Once that is done, the review phase can commence.

BLuedtke commented 2 years ago

TODO before merge: