Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop usage of Boost Thread for the macosx target and remove ORO_OS_USE_BOOST_THREAD #310

Open
wants to merge 7 commits into
base: toolchain-2.9
Choose a base branch
from

Conversation

meyerj
Copy link
Member

@meyerj meyerj commented Oct 24, 2019

Addresses #53 (comment):

This patch and also the original version that defines BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG in public headers is invalid.

BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG is a compile-time option of Boost, which should not be defined in a header file. You might get away with defining it in a cpp file and only use Boost header-only functions. But other libraries like Boost.Thread or string formatting in Boost.DateTime are typically pre-compiled and the differences of the internal representation of boost::posix_time::ptime caused by defining BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG might cause run-time errors or unexpected behavior if the user mixes RTT code with calls into one of those Boost libraries.

See http://www.boost.org/doc/libs/1_66_0/more/getting_started/unix-variants.html#header-only-libraries for an overview of which Boost libraries are header-only.

Boost.Thread was used to implement mutexes and condition variables for the macOS target (macosx). The main reason for this was that Apple's implementation of pthread_mutex_t does not have pthread_mutex_timedlock. Workaround: Pair the mutex with a condition variable that is signaled whenever the mutex is unlocked. This is also how boost::timed_mutex is implemented for the case BOOST_THREAD_USES_PTHREAD_TIMEDLOCK is not defined (cf. https://github.com/boostorg/thread/blob/boost-1.62.0/include/boost/thread/pthread/mutex.hpp#L181).

TLSF still uses pthread_mutex_t directly for efficiency.

There was also a bug in TLSF 2.4.6 that was fixed in 3ffcc5e:
The calculation of the maximum size of the area in init_memory_pool() was wrong for certain combinations of mem_size_pool and sizeof(tlsf_t), where

ROUNDUP_SIZE(sizeof(tlsf_t)) + ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) > mem_pool_size

. This caused an invalid write after the pool in process_area().

Example:

mem_pool_size = 15000;
sizeof(tlsf_t) = 6392;
BLOCK_ALIGN = 16

==> ROUNDUP_SIZE(sizeof(tlsf_t)) = 6400
    ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) = 8608
    ROUNDUP_SIZE(sizeof(tlsf_t))) + ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) = 15008

The fix is to add another ROUNDUP_SIZE() macro call in the second summand:

ROUNDUP_SIZE(sizeof(tlsf_t))) + ROUNDDOWN_SIZE(mem_pool_size - ROUNDUP_SIZE(sizeof(tlsf_t))) = 14992

….h that breaks linkage with Boost

This patch reverts 9b0c9bc, ab8ddf7,
and parts of 4aa8567.

BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG is a compile-time option of Boost, which should not be defined in
a RTT header file. User libraries like Boost.Thread or string formatting in Boost.DateTime are typically
pre-compiled and the differences of the internal representation of boost::posix_time::ptime caused by
defining BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG might cause run-time errors or unexpected behavior if the
user mixes RTT code with calls into one of those Boost libraries.

Boost.Thread converts the ptime instance in a struct timespec internally.

Signed-off-by: Johannes Meyer <[email protected]>
… variables for the macosx target

The only reason why the dependency to Boost Thread was introduced in d3b21ae
was because pthread_mutex_timedlock() is not available in Mac OS. Boost Thread internally combines a pthread
mutex with a condition variable to implement timed_mutex and recursive_timed_mutex, which is what this patch
applies directly in the C implementation of rt_mutex_t and rt_rec_mutex_t.

Signed-off-by: Johannes Meyer <[email protected]>
The calculation of the maximum size of the area in init_memory_pool() was wrong for certain combinations
of mem_size_pool and sizeof(tlsf_t), where

ROUNDUP_SIZE(sizeof(tlsf_t)) + ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) > mem_pool_size

. This caused an invalid write after the pool in process_area().

Example:

mem_pool_size = 15000;
sizeof(tlsf_t) = 6392;
BLOCK_ALIGN = 16

==> ROUNDUP_SIZE(sizeof(tlsf_t)) = 6400
    ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) = 8608
    ROUNDUP_SIZE(sizeof(tlsf_t))) + ROUNDDOWN_SIZE(mem_pool_size - sizeof(tlsf_t)) = 15008

The fix is to add another ROUNDUP_SIZE() macro call in the second summand:

ROUNDUP_SIZE(sizeof(tlsf_t))) + ROUNDDOWN_SIZE(mem_pool_size - ROUNDUP_SIZE(sizeof(tlsf_t))) = 14992

Signed-off-by: Johannes Meyer <[email protected]>
… a timed mutex implementation

Signed-off-by: Johannes Meyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant