Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ set(rcutils_sources
src/strerror.c
src/string_array.c
src/string_map.c
src/testing/fault_injection.c
src/time.c
${time_impl_c}
src/uint8_array.c
Expand Down Expand Up @@ -105,6 +106,10 @@ target_include_directories(${PROJECT_NAME} PUBLIC
# which is appropriate when building the dll but not consuming it.
target_compile_definitions(${PROJECT_NAME} PRIVATE "RCUTILS_BUILDING_DLL")

if(BUILD_TESTING AND NOT RCUTILS_DISABLE_FAULT_INJECTION)
target_compile_definitions(${PROJECT_NAME} PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

target_link_libraries(${PROJECT_NAME} ${CMAKE_DL_LIBS})

# Needed if pthread is used for thread local storage.
Expand Down
1 change: 1 addition & 0 deletions Doxyfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ EXPAND_ONLY_PREDEF = YES
PREDEFINED += RCUTILS_PUBLIC=
PREDEFINED += RCUTILS_PUBLIC_TYPE=
PREDEFINED += RCUTILS_WARN_UNUSED=
PREDEFINED += RCUTILS_ENABLE_FAULT_INJECTION=

# 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 include/rcutils/error_handling.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ extern "C"
#include "rcutils/allocator.h"
#include "rcutils/macros.h"
#include "rcutils/snprintf.h"
#include "rcutils/testing/fault_injection.h"
#include "rcutils/types/rcutils_ret.h"
#include "rcutils/visibility_control.h"

Expand Down
69 changes: 69 additions & 0 deletions include/rcutils/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,75 @@ extern "C"
# define RCUTILS_UNLIKELY(x) (x)
#endif // _WIN32

#if defined RCUTILS_ENABLE_FAULT_INJECTION
#include "rcutils/testing/fault_injection.h"

/**
* \def RCUTILS_CAN_RETURN_WITH_ERROR_OF
* Indicating macro that the function intends to return possible error value.
*
* Put this macro as the first line in the function. For example:
*
* int rcutils_function_that_can_fail() {
* RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_INVALID_ARGUMENT);
* ... // rest of function
* }
*
* For now, this macro just simply calls `RCUTILS_FAULT_INJECTION_MAYBE_RETURN_ERROR` if fault
* injection is enabled. However, for source code, the macro annotation
* `RCUTILS_CAN_RETURN_WITH_ERROR_OF` helps clarify that a function may return a value signifying
* an error and what those are.
*
* In general, you should only include a return value that originates in the function you're
* annotating instead of one that is merely passed on from a called function already annotated with
*`RCUTILS_CAN_RETURN_WITH_ERROR_OF`. If you are passing on return values from a called function,
* but that function is not annotated with `RCUTILS_CAN_RETURN_WITH_ERROR_OF`, then you might
* consider annotating that function first. If for some reason that is not desired or possible,
* then annotate your function as if the return values you are passing on originated from your
* function.
*
* If the function can return multiple return values indicating separate failure types, each one
* should go on a separate line.
*
* If in your function, there are expected effects on output parameters that occur during
* the failure case, then it will introduce a discrepancy between fault injection testing and
* production operation. This is because the fault injection will cause the function to return
* where this macro is used, not at the location the error values are typically returned. To help
* protect against this scenario you may consider adding unit tests that check your function does
* not modify output parameters when it actually returns a failing error code if it's possible for
* your code.
*
* If your function is void, this macro can be used without parameters. However, for the above
* reasoning, there should be no side effects on output parameters for all possible early returns.
*
* \param error_return_value the value returned as a result of an error. It does not need to be
* a rcutils_ret_t type. It could also be NULL, -1, a string error message, etc
*/
# define RCUTILS_CAN_RETURN_WITH_ERROR_OF(error_return_value) \
RCUTILS_FAULT_INJECTION_MAYBE_RETURN_ERROR(error_return_value);

/**
* \def RCUTILS_CAN_FAIL_WITH
* Indicating macro similar to RCUTILS_CAN_RETURN_WITH_ERROR_OF but for use with more complicated
* statements.
*
* The `failure_code` will be executed inside a scoped if block, so any variables declared within
* will not be available outside of the macro.
*
* One example where you might need this version, is if a side-effect may occur within a function.
* For example, in snprintf, rcutils_snprintf needs to set both errno and return -1 on failure.
* This macro is used to capture both effects.
*
* \param failure_code Code that is representative of the failure case in this function.
*/
# define RCUTILS_CAN_FAIL_WITH(failure_code) \
RCUTILS_FAULT_INJECTION_MAYBE_FAIL(failure_code);

#else
# define RCUTILS_CAN_RETURN_WITH_ERROR_OF(error_return_value)
# define RCUTILS_CAN_FAIL_WITH(failure_code)
#endif // defined RCUTILS_ENABLE_FAULT_INJECTION

#ifdef __cplusplus
}
#endif
Expand Down
174 changes: 174 additions & 0 deletions include/rcutils/testing/fault_injection.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
// Copyright 2020 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 RCUTILS__TESTING__FAULT_INJECTION_H_
#define RCUTILS__TESTING__FAULT_INJECTION_H_
#include <stdbool.h>
#include <stdio.h>
#include <stdint.h>

#include "rcutils/macros.h"
#include "rcutils/visibility_control.h"

#ifdef __cplusplus
extern "C"
{
#endif

#define RCUTILS_FAULT_INJECTION_NEVER_FAIL -1

#define RCUTILS_FAULT_INJECTION_FAIL_NOW 0

RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
bool
rcutils_fault_injection_is_test_complete(void);

/**
* \brief Atomically set the fault injection counter.
*
* This is typically not the preferred method of interacting directly with the fault injection
* logic, instead use `RCUTILS_FAULT_INJECTION_TEST` instead.
*
* This function may also be used for pausing code inside of a `RCUTILS_FAULT_INJECTION_TEST` with
* something like the following:
*
* RCUTILS_FAULT_INJECTION_TEST({
* ... // code to run with fault injection
* int64_t count = rcutils_fault_injection_get_count();
* rcutils_fault_injection_set_count(RCUTILS_FAULT_INJECTION_NEVER_FAIL);
* ... // code to run without fault injection
* rcutils_fault_injection_set_count(count);
* ... // code to run with fault injection
* });
*
* \param count The count to set the fault injection counter to. If count is negative, then fault
* injection errors will be disabled. The counter is globally initialized to
* RCUTILS_FAULT_INJECTION_NEVER_FAIL.
*/
RCUTILS_PUBLIC
void
rcutils_fault_injection_set_count(int count);

/**
* \brief Atomically get the fault injection counter value
*
* This function is typically not used directly but instead indirectly inside an
* `RCUTILS_FAULT_INJECTION_TEST`
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
int_least64_t
rcutils_fault_injection_get_count(void);

/**
* \brief Implementation of fault injection decrementer
*
* This is included inside of macros, so it needs to be exported as a public function, but it
* should not be used directly.
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
int_least64_t
_rcutils_fault_injection_maybe_fail(void);

/**
* \def RCUTILS_FAULT_INJECTION_MAYBE_RETURN_ERROR
* \brief This macro checks and decrements a static global variable atomic counter and returns
* `return_value_on_error` if 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

@brawner one thing this scheme cannot do is force an error during cleanup after one or more errors have been forced. What about if, in addition to forcing a single function to fail, we make all following functions fail as well? Like a flag to prevent the count to ever be decremented below 0.

I envision the RCUTILS_CHECK_FAULT_SAFETY macro below taking an optional FLUKE or MASSIVE argument for each case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some examples where a failure in one location is likely to result in a failure in another location. For example, allocations may continue to fail, or file system functions may continue to fail. For the sake of coverage, there are locations where it would take two failures to reach some lines of code. Your example of cleanup is a good one.

I think there are a few of potential solutions:

  1. Setting potential alternate behaviors when the counter is negative, which is what you are suggesting. I think the idea works great, it would just require writing multiple unit tests for each behavior.

  2. Instead of leaving it as -1, the RCUTILS_SET_FAULT_COUNT macro could have a variadic version which takes as parameters counts that it's reset to after it reaches 0.

RCUTILS_SET_FAULT_COUNT(0, 0)

For example, would trip a second failure immediately after its first failure. This would let you test more types of failures, and then we could use this type of macro to check that code is not only one-fault tolerant, but two-fault tolerant, three-fault tolerant etc.

  1. For instances where testing two particular faults is important, then we could introduce a named fault injection. Instead of RCUTILS_CAN_RETURN_WITH_ERROR, there would be a RCUTILS_CAN_RETURN_WITH_ERROR_NAMED. Then in the unit test, we could target those locations to fail explicitly in a unit test.
RCUTILS_SET_FAULT_INJECTION_NAMED(...);
RCUTILS_SET_FAULT_INJECTION_NAMED(...);
... // Run test code

If we do introduce this one at a later date, I could see it replacing the unnamed version macro entirely. This idea requires multiple unit tests for each targeted failed injection, but is also the only option that allows for targeted injections in the first place.

Do you have any thoughts about the other options suitability for the situation you are describing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think solution (2.) would be best. We don't really know what we're hitting, so N-fault tolerant sounds as good as it gets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be added in a followup PR when the functionality becomes necessary for better coverage.

*
* This macro is not a function itself, so when this macro returns it will cause the calling
* function to return with the return value.
*
* Set the counter with `RCUTILS_FAULT_INJECTION_SET_COUNT`. If the count is less than 0, then
* `RCUTILS_FAULT_INJECTION_MAYBE_RETURN_ERROR` will not cause an early return.
*
* This macro is thread-safe, and ensures that at most one invocation results in a failure for each
* time the fault injection counter is set with `RCUTILS_FAULT_INJECTION_SET_COUNT`
*
* \param return_value_on_error the value to return in the case of fault injected failure.
*/
#define RCUTILS_FAULT_INJECTION_MAYBE_RETURN_ERROR(return_value_on_error) \
if (RCUTILS_FAULT_INJECTION_FAIL_NOW == _rcutils_fault_injection_maybe_fail()) { \
printf( \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been a very helpful error message to print out while debugging tests. Is there not a way to use a logging macro here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see why not having an RCUTILS_LOG_DEBUG call here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RCUTILS_LOG_DEBUG is not defined inside the rcutils library unfortunately.

"%s:%d Injecting fault and returning " #return_value_on_error "\n", __FILE__, __LINE__); \
return return_value_on_error; \
}

/**
* \def RCUTILS_FAULT_INJECTION_MAYBE_FAIL
* \brief This macro checks and decrements a static global variable atomic counter and executes
* `failure_code` if the counter is 0 inside a scoped block (any variables declared in
* failure_code) will not be avaliable outside of this scoped block.
*
* This macro is not a function itself, so it will cause the calling function to execute the code
* from within an if loop.
*
* Set the counter with `RCUTILS_FAULT_INJECTION_SET_COUNT`. If the count is less than 0, then
* `RCUTILS_FAULT_INJECTION_MAYBE_FAIL` will not execute the failure code.
*
* This macro is thread-safe, and ensures that at most one invocation results in a failure for each
* time the fault injection counter is set with `RCUTILS_FAULT_INJECTION_SET_COUNT`
*
* \param failure_code the code to execute in the case of fault injected failure.
*/
#define RCUTILS_FAULT_INJECTION_MAYBE_FAIL(failure_code) \
if (RCUTILS_FAULT_INJECTION_FAIL_NOW == _rcutils_fault_injection_maybe_fail()) { \
printf( \
"%s:%d Injecting fault and executing " #failure_code "\n", __FILE__, __LINE__); \
failure_code; \
}

/**
* \def RCUTILS_FAULT_INJECTION_TEST
*
* The fault injection macro for use with unit tests to check that `code` can tolerate injected
* failures at all points along the execution path where the indicating macros
* `RCUTILS_CAN_RETURN_WITH_ERROR_OF` and `RCUTILS_CAN_FAIL_WITH` are located.
*
* This macro is intended to be used within a gtest function macro like 'TEST', 'TEST_F', etc.
*
* `code` is executed within a do-while loop and therefore any variables declared within are in
* their own scope block.
*
* Here's a simple example:
* RCUTILS_FAULT_INJECTION_TEST(
* rcl_ret_t ret = rcl_init(argc, argv, options, context);
* if (RCL_RET_OK == ret)
* {
* ret = rcl_shutdown(context);
* }
* });
*
* In this example, you will need have conditional execution based on the return value of
* `rcl_init`. If it failed, then it wouldn't make sense to call rcl_shutdown. In your own test,
* there might be similar logic that requires conditional checks. The goal of writing this test
* is less about checking the behavior is consistent, but instead that failures do not cause
* program crashes, memory errors, or unnecessary memory leaks.
*/
#define RCUTILS_FAULT_INJECTION_TEST(code) \
do { \
int fault_injection_count = 0; \
do { \
rcutils_fault_injection_set_count(fault_injection_count++); \
code; \
Copy link
Contributor

Choose a reason for hiding this comment

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

@brawner nice! I still think we need a way to disable fault injection temporarily within the code statement, otherwise you may hit a fault while handling or checking the outcome of the piece of functionality that is actually under test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That can be done by way of:

int current_count = RCUTILS_FAULT_INJECTION_GET_COUNT();
RCUTILS_FAULT_INJECTION_SET_COUNT(RCUTILS_FAULT_INJECTION_NEVER_FAIL);
...
RCUTILS_FAULT_INJECTION_SET_COUNT(current_count);

Do you think we need something more like:

RCUTILS_FAULT_INJECTION_PAUSE_TEST();
...
RCUTILS_FAULT_INJECTION_RESUME_TEST();

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, some sugar would be nice. I'd personally like to have something along the lines of:

RCUTILS_FAULT_INJECTION_NO_INJECT({
  // ...
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think your solution will introduce scoping issues, but I still agree that a potential use case along these lines may be useful. As I currently don't need it for the existing packages, I'll save this for a followup PR.

} while (!rcutils_fault_injection_is_test_complete()); \
rcutils_fault_injection_set_count(RCUTILS_FAULT_INJECTION_NEVER_FAIL); \
} while (0)

#ifdef __cplusplus
}
#endif

#endif // RCUTILS__TESTING__FAULT_INJECTION_H_
5 changes: 5 additions & 0 deletions src/shared_library.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ C_ASSERT(sizeof(void *) == sizeof(HINSTANCE));
#endif // _WIN32

#include "rcutils/error_handling.h"
#include "rcutils/macros.h"
#include "rcutils/shared_library.h"
#include "rcutils/strdup.h"

Expand All @@ -46,6 +47,10 @@ rcutils_load_shared_library(
const char * library_path,
rcutils_allocator_t allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_BAD_ALLOC);
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_ERROR);

RCUTILS_CHECK_ARGUMENT_FOR_NULL(lib, RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ARGUMENT_FOR_NULL(library_path, RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ALLOCATOR(&allocator, return RCUTILS_RET_INVALID_ARGUMENT);
Expand Down
2 changes: 2 additions & 0 deletions src/snprintf.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ rcutils_snprintf(char * buffer, size_t buffer_size, const char * format, ...)
int
rcutils_vsnprintf(char * buffer, size_t buffer_size, const char * format, va_list args)
{
RCUTILS_CAN_FAIL_WITH({errno = EINVAL; return -1;});

if (NULL == format) {
errno = EINVAL;
return -1;
Expand Down
6 changes: 6 additions & 0 deletions src/strdup.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ extern "C"
#include <string.h>

#include "./common.h"
#include "rcutils/macros.h"


char *
rcutils_strdup(const char * str, rcutils_allocator_t allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(NULL);

if (NULL == str) {
return NULL;
}
Expand All @@ -36,6 +40,8 @@ rcutils_strdup(const char * str, rcutils_allocator_t allocator)
char *
rcutils_strndup(const char * str, size_t string_length, rcutils_allocator_t allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(NULL);

if (NULL == str) {
return NULL;
}
Expand Down
5 changes: 5 additions & 0 deletions src/string_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ rcutils_string_array_init(
size_t size,
const rcutils_allocator_t * allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_BAD_ALLOC);

if (NULL == allocator) {
RCUTILS_SET_ERROR_MSG("allocator is null");
return RCUTILS_RET_INVALID_ARGUMENT;
Expand All @@ -63,6 +66,8 @@ rcutils_string_array_init(
rcutils_ret_t
rcutils_string_array_fini(rcutils_string_array_t * string_array)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCUTILS_RET_INVALID_ARGUMENT);

if (NULL == string_array) {
RCUTILS_SET_ERROR_MSG("string_array is null");
return RCUTILS_RET_INVALID_ARGUMENT;
Expand Down
Loading