Skip to content

Commit

Permalink
Revert "Ensure typesupport handle functions do not throw. (#99)"
Browse files Browse the repository at this point in the history 
This reverts commit e513ffb.
  • Loading branch information
@clalancette
clalancette committed Dec 15, 2020
1 parent e513ffb commit 4401542
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 91 deletions.
22 changes: 9 additions & 13 deletions rosidl_typesupport_c/src/type_support_dispatch.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@

#include <memory>
#include <stdexcept>
#include <list>
#include <string>

#include "rcpputils/find_library.hpp"
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rcutils/snprintf.h"
#include "rosidl_typesupport_c/identifier.h"
#include "rosidl_typesupport_c/type_support_map.h"

Expand Down Expand Up @@ -55,39 +55,35 @@ get_typesupport_handle_function(

if (!map->data[i]) {
char library_name[1024];
int ret = rcutils_snprintf(
snprintf(
library_name, 1023, "%s__%s",
map->package_name, identifier);
if (ret < 0) {
RCUTILS_SET_ERROR_MSG("Failed to format library name");
return nullptr;
}

std::string library_path;
try {
library_path = rcpputils::find_library_path(library_name);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s' due to %s",
"Failed to find library '%s' due to %s\n",
library_name, e.what());
return nullptr;
}

if (library_path.empty()) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s'", library_name);
"Failed to find library '%s'\n", library_name);
return nullptr;
}

try {
lib = new rcpputils::SharedLibrary(library_path.c_str());
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_path.c_str(), e.what());
"Could not load library %s: %s\n", library_path.c_str(), e.what());
return nullptr;
} catch (const std::bad_alloc & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_path.c_str(), e.what());
"Could not load library %s: %s\n", library_path.c_str(), e.what());
return nullptr;
}
map->data[i] = lib;
Expand All @@ -100,13 +96,13 @@ get_typesupport_handle_function(
try {
if (!lib->has_symbol(map->symbol_name[i])) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find symbol '%s' in library", map->symbol_name[i]);
"Failed to find symbol '%s' in library\n", map->symbol_name[i]);
return nullptr;
}
sym = lib->get_symbol(map->symbol_name[i]);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to get symbol '%s' in library: %s",
"Failed to get symbol '%s' in library: %s\n",
map->symbol_name[i], e.what());
return nullptr;
}
Expand All @@ -118,7 +114,7 @@ get_typesupport_handle_function(
}
}
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Handle's typesupport identifier (%s) is not supported by this library",
"Handle's typesupport identifier (%s) is not supported by this library\n",
handle->typesupport_identifier);
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/include/rosidl_typesupport_cpp/message_type_support_dispatch.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace rosidl_typesupport_cpp
ROSIDL_TYPESUPPORT_CPP_PUBLIC
const rosidl_message_type_support_t *
get_message_typesupport_handle_function(
const rosidl_message_type_support_t * handle, const char * identifier) noexcept;
const rosidl_message_type_support_t * handle, const char * identifier);

} // namespace rosidl_typesupport_cpp

Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/include/rosidl_typesupport_cpp/service_type_support_dispatch.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace rosidl_typesupport_cpp
ROSIDL_TYPESUPPORT_CPP_PUBLIC
const rosidl_service_type_support_t *
get_service_typesupport_handle_function(
const rosidl_service_type_support_t * handle, const char * identifier) noexcept;
const rosidl_service_type_support_t * handle, const char * identifier);

} // namespace rosidl_typesupport_cpp

Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/src/message_type_support_dispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace rosidl_typesupport_cpp

const rosidl_message_type_support_t *
get_message_typesupport_handle_function(
const rosidl_message_type_support_t * handle, const char * identifier) noexcept
const rosidl_message_type_support_t * handle, const char * identifier)
{
return rosidl_typesupport_cpp::get_typesupport_handle_function<
rosidl_message_type_support_t>(handle, identifier);
Expand Down
2 changes: 1 addition & 1 deletion rosidl_typesupport_cpp/src/service_type_support_dispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace rosidl_typesupport_cpp

const rosidl_service_type_support_t *
get_service_typesupport_handle_function(
const rosidl_service_type_support_t * handle, const char * identifier) noexcept
const rosidl_service_type_support_t * handle, const char * identifier)
{
return rosidl_typesupport_cpp::get_typesupport_handle_function<
rosidl_service_type_support_t>(handle, identifier);
Expand Down
58 changes: 18 additions & 40 deletions rosidl_typesupport_cpp/src/type_support_dispatch.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
#include <cstring>
#include <memory>
#include <stdexcept>

#include <string>

#include "rcpputils/find_library.hpp"
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rcutils/snprintf.h"
#include "rosidl_typesupport_c/type_support_map.h"

namespace rosidl_typesupport_cpp
Expand All @@ -36,7 +35,7 @@ extern const char * typesupport_identifier;
template<typename TypeSupport>
const TypeSupport *
get_typesupport_handle_function(
const TypeSupport * handle, const char * identifier) noexcept
const TypeSupport * handle, const char * identifier)
{
if (strcmp(handle->typesupport_identifier, identifier) == 0) {
return handle;
Expand All @@ -52,72 +51,51 @@ get_typesupport_handle_function(
rcpputils::SharedLibrary * lib = nullptr;

if (!map->data[i]) {
char library_name[1024];
int ret = rcutils_snprintf(
library_name, 1023, "%s__%s",
map->package_name, identifier);
if (ret < 0) {
RCUTILS_SET_ERROR_MSG("Failed to format library name");
return nullptr;
}
std::string library_name{map->package_name};
library_name += "__";
library_name += identifier;

std::string library_path;
try {
library_path = rcpputils::find_library_path(library_name);
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s' due to %s",
library_name, e.what());
return nullptr;
const std::string message =
"Failed to find library '" + library_name + "' due to " + e.what();
throw std::runtime_error(message);
}

if (library_path.empty()) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find library '%s'", library_name);
fprintf(stderr, "Failed to find library '%s'\n", library_name.c_str());
return nullptr;
}

try {
lib = new rcpputils::SharedLibrary(library_path.c_str());
} catch (const std::runtime_error & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_name, e.what());
return nullptr;
throw std::runtime_error(
"Could not load library " + library_path + ": " +
std::string(e.what()));
} catch (const std::bad_alloc & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Could not load library %s: %s", library_name, e.what());
return nullptr;
throw std::runtime_error(
"Could not load library " + library_path + ": " +
std::string(e.what()));
}
map->data[i] = lib;
}
auto clib = static_cast<const rcpputils::SharedLibrary *>(map->data[i]);
lib = const_cast<rcpputils::SharedLibrary *>(clib);

void * sym = nullptr;

try {
if (!lib->has_symbol(map->symbol_name[i])) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to find symbol '%s' in library", map->symbol_name[i]);
return nullptr;
}
sym = lib->get_symbol(map->symbol_name[i]);
} catch (const std::exception & e) {
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Failed to get symbol '%s' in library: %s",
map->symbol_name[i], e.what());
if (!lib->has_symbol(map->symbol_name[i])) {
fprintf(stderr, "Failed to find symbol '%s' in library\n", map->symbol_name[i]);
return nullptr;
}
void * sym = lib->get_symbol(map->symbol_name[i]);

typedef const TypeSupport * (* funcSignature)(void);
funcSignature func = reinterpret_cast<funcSignature>(sym);
const TypeSupport * ts = func();
return ts;
}
}
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Handle's typesupport identifier (%s) is not supported by this library",
handle->typesupport_identifier);
return nullptr;
}

Expand Down
27 changes: 10 additions & 17 deletions rosidl_typesupport_cpp/test/test_message_type_support_dispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/error_handling.h"
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_c/type_support_map.h"
Expand Down Expand Up @@ -85,8 +84,6 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_message_type_support_t type_support_cpp_identifier =
get_rosidl_message_type_support(rosidl_typesupport_cpp::typesupport_identifier);
Expand Down Expand Up @@ -116,24 +113,18 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support2"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library file exists, but loading shared library fails
EXPECT_EQ(
EXPECT_THROW(
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support3"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
"test_type_support3"), std::runtime_error);

// Library doesn't exist
EXPECT_EQ(
rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
Expand All @@ -147,12 +138,14 @@ TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
auto * result = rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
try {
auto * result = rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
ADD_FAILURE() << "Unexpected exception type in fault injection test";
}
});
}
26 changes: 9 additions & 17 deletions rosidl_typesupport_cpp/test/test_service_type_support_dispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/error_handling.h"
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_c/type_support_map.h"
Expand Down Expand Up @@ -85,8 +84,6 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support,
"different_identifier"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

rosidl_service_type_support_t type_support_cpp_identifier =
get_rosidl_service_type_support(rosidl_typesupport_cpp::typesupport_identifier);
Expand All @@ -113,24 +110,18 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support2"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();

// Library file exists, but loading shared library fails
EXPECT_EQ(
EXPECT_THROW(
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support3"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
"test_type_support3"), std::runtime_error);

// Library doesn't exist
EXPECT_EQ(
rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
Expand All @@ -144,12 +135,13 @@ TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
auto * result = rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
try {
auto * result = rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
}
});
}

0 comments on commit 4401542

Please sign in to comment.