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

Avoid priority inversions when using FIFO scheduling #174

Open
wants to merge 17 commits into
base: rolling
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ add_library(${PROJECT_NAME}
src/filesystem_helper.cpp
src/find_library.cpp
src/env.cpp
src/mutex.cpp
src/shared_library.cpp)
target_include_directories(${PROJECT_NAME} PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
Expand All @@ -36,6 +37,16 @@ if(WIN32)
target_compile_definitions(${PROJECT_NAME}
PRIVATE "RCPPUTILS_BUILDING_LIBRARY")
endif()

option(USE_PI_MUTEX "Enables priority inheritance mutexes." ON)
if(USE_PI_MUTEX)
if(WIN32 OR APPLE)
Copy link

@carlossvg carlossvg Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do it the other way around. I would list systems with PI_MUTEX support and fail if the system is not in the list. With this approach you will get an error for unknown systems and for new systems you explicitly add them when you know it is supported.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Better now?

Unfortunately I could not find much info on the CMAKE_SYSTEM_NAME variable for VxWorks and QNX systems. I hope I got the right strings.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@razr FYI

message("Mutexes with priority inheritance are not supported on your system. Using std mutexes.")
else()
target_compile_definitions(${PROJECT_NAME} PUBLIC "RCPPUTILS_USE_PIMUTEX")
endif()
endif()

ament_target_dependencies(${PROJECT_NAME} rcutils)

# Export old-style CMake variables
Expand Down Expand Up @@ -73,6 +84,9 @@ if(BUILD_TESTING)
ament_add_gtest(test_join test/test_join.cpp)
target_link_libraries(test_join ${PROJECT_NAME})

ament_add_gtest(test_mutex test/test_mutex.cpp)
target_link_libraries(test_mutex ${PROJECT_NAME})

ament_add_gtest(test_time test/test_time.cpp)
target_link_libraries(test_time ${PROJECT_NAME})
ament_target_dependencies(test_time rcutils)
Expand Down
1 change: 1 addition & 0 deletions Doxyfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ EXPAND_ONLY_PREDEF = YES
PREDEFINED += __declspec(x)=
PREDEFINED += RCPPUTILS_PUBLIC=
PREDEFINED += RCPPUTILS_PUBLIC_TYPE=
PREDEFINED += RCPPUTILS_USE_PIMUTEX

# Tag files that do not exist will produce a warning and cross-project linking will not work.
TAGFILES += "../../../doxygen_tag_files/cppreference-doxygen-web.tag.xml=http://en.cppreference.com/w/"
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ This package currently contains:
* String helpers
* File system helpers
* Type traits helpers
* Mutex classes that support thread priority inheritance
* Class that dynamically loads, unloads and get symbols from shared libraries at run-time.

Features are described in more detail at [docs/FEATURES.md](docs/FEATURES.md)
68 changes: 68 additions & 0 deletions include/rcpputils/mutex.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2023 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef RCPPUTILS__MUTEX_HPP_
#define RCPPUTILS__MUTEX_HPP_

#include <mutex>

#include "rcpputils/visibility_control.hpp"

namespace rcpputils
{
#ifndef RCPPUTILS_USE_PIMUTEX

// Fallback code path
using PIMutex = std::mutex;
using RecursivePIMutex = std::recursive_mutex;

#else

/**
* Mutex with priority inheritance on systems that support it.
* This class derives from std::mutex to be fully compatible with standard C++.
* This implementation is needed because the C++ standard library doesn't support priority inheritance.
**/
class PIMutex : public std::mutex
{
public:
/// Creates a mutex with priority inheritance enabled
RCPPUTILS_PUBLIC
PIMutex();

/// Releases the mutex
RCPPUTILS_PUBLIC
~PIMutex();
};

/**
* Recursive mutex with priority inheritance on systems that support it.
* This class derives from std::recursive_mutex to be fully compatible with standard C++.
* This implementation is needed because the C++ standard library doesn't support priority inheritance.
**/
class RecursivePIMutex : public std::recursive_mutex
{
public:
/// Creates a recursive mutex with priority inheritance enabled
RCPPUTILS_PUBLIC
RecursivePIMutex();

/// Releases the mutex
RCPPUTILS_PUBLIC
~RecursivePIMutex();
};

#endif // RCPPUTILS_USE_PIMUTEX
} // namespace rcpputils
#endif // RCPPUTILS__MUTEX_HPP_
72 changes: 72 additions & 0 deletions src/mutex.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2023 Open Source Robotics Foundation, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rcpputils/mutex.hpp"

#ifdef RCPPUTILS_USE_PIMUTEX

#include <pthread.h>

namespace rcpputils
{

PIMutex::PIMutex()
{
// Destroy the underlying mutex
pthread_mutex_destroy(native_handle());

// Create mutex attribute with desired settings
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT);

// Initialize the underlying mutex
if (pthread_mutex_init(native_handle(), &attr) != 0) {
throw std::runtime_error("Mutex initialization failed.");
}

// The attribute object isn't needed any more
pthread_mutexattr_destroy(&attr);
}

RecursivePIMutex::RecursivePIMutex()
{
// Destroy the underlying mutex
pthread_mutex_destroy(native_handle());

// Create mutex attribute with desired settings
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);

// Initialize the underlying mutex
if (pthread_mutex_init(native_handle(), &attr) != 0) {
throw std::runtime_error("Recursive mutex initialization failed.");
}

// The attribute object isn't needed any more
pthread_mutexattr_destroy(&attr);
}

PIMutex::~PIMutex()
{
}

RecursivePIMutex::~RecursivePIMutex()
{
}

} // namespace rcpputils
#endif // RCPPUTILS_USE_PIMUTEX
Loading