pybind breaks exception handling

[wichert]

I've spent many hours today trying to figure out why my exception handling does not work. I have a C++ library which raises an exception in its own namespace: stylist::unsuitable_error. Catching this error works perfectly within that library. In my wrapper code it fails. The exception is defined like this:

class unsuitable_error : public std::logic_error {
public:
	explicit unsuitable_error(const std::string& __arg) : std::logic_error(__arg), reasons(), causes() { }
	virtual ~unsuitable_error() throw() { }

	std::list<const char*> reasons;
	struct cause_type { … };
	std::list<cause_type> causes;
};

I have some code which tries to handle that exception being thrown:

	AdviceWrapper* doit(const Article& article, const Body& body) {
		auto result = new AdviceWrapper();

		try {
			result->advise = advise(article, body);
			cerr << "All OK" << endl;
		} catch (const unsuitable_error &e) {
			cerr << "Not suitable" << endl;
		} catch (...) {
			cerr << "Bad exception" << endl;
		}
		return result;
	}

void GO() {
  article = … ;
  body = … ;
  doit(article, body);
}

This works perfectly in a standalone program:

$ cat src/standalone.cc 
void GO();

int main() {
	GO();
	return 0;
}
$ ./standalone 
Not suitable

However, the exact same code in pybind11 breaks. The wrapper code is simple enough:

#include <pybind11/pybind11.h>

namespace py = pybind11;


void GO();

PYBIND11_MODULE(_stylist, m) {
	m.doc() = "CurveTips stylist";

	m.def("GO", GO);
}

and running that results in:

$ python3 -c 'import _stylist ; _stylist.GO()'
Bad exception

The standalone tool and Python wrapper are compiled from the same CMake config:

project(_stylist)
cmake_minimum_required(VERSION 2.8.12)

set(PACKAGE stylist)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
set(Boost_USE_MULTITHREADED ON)

find_package(Stylist REQUIRED)
include_directories(SYSTEM ${Stylist_INCLUDE_DIRS})
link_directories(${Stylist_LIBRARY_DIRS})

add_subdirectory(pybind11)
pybind11_add_module(_stylist SHARED src/stylist.cc src/tst.cc)
target_link_libraries(_stylist PRIVATE ${Stylist_LIBRARIES})

add_executable(standalone src/standalone.cc src/tst.cc)
target_link_libraries(standalone PRIVATE ${Stylist_LIBRARIES})

[jagerman]

This is most likely caused by -fvisibility=hidden: the two exception classes, despite having the same name, are being treated as independent classes by gcc. (Before you ask, turning off -fvisibility=hidden isn't really an option: we'd instantly hit clashes when loading multiple modules into the same interpreter if the pybind versions weren't exactly identical).

Try, as a quick test, adding PYBIND11_EXPORT into your exception definition:

class PYBIND11_EXPORT unsuitable_error : public std::logic_error { ... }

or if including pybind/pybind11.h is a nuissance, just expand it manually:

class __attribute__ ((visibility("default"))) unsuitable_error : public std::logic_error { ... }

If that fixes it, then it's definitely identified the issue. From there, it's a matter of figuring out how to solve it (without needing to change the foreign header). A couple ideas to try (I haven't tested these):

• before you include the header that defines the problematic class from the binding code, try forward-declaring the class with the export macro, i.e.:

class PYBIND11_EXPORT unsuitable_error;
#include <whatever.h>

• Alternatively, try surrounding the include itself in a non-hidden visibility pragma block:

#pragma GCC visibility push(default)
#include <whatever.h>
#pragma GCC visibility pop

#include <pybind11/pybind11.h>

/// binding code

[wichert]

@jagerman That indeed fixes it! Both workarounds you propose work.

Is there anything specific that makes my exception class problematic (compared to the stdexcept classes for example), or is this is a generic issue that more people are likely to hit?

[jagerman]

Actually, the part about -fvisibility=hidden being required might not be true anymore (as of #995 / 2.2.0): we now force hidden visibility on everything in the pybind11 namespace (since #995), so it should actually be safe to compile modules without -fvisibility=hidden. So that's another option to consider (by changing the cmake CXX_VISIBILITY_PRESET for the target to default rather than hidden that the default pybind cmake code sets).

[wichert]

Adding set(CMAKE_CXX_VISIBILITY_PRESET default) does not seem to help.

[wichert]

Oh, that probably should be set_target_properties(${target_name} PROPERTIES CXX_VISIBILITY_PRESET "default"), after pybind11_add_module? That seems to work fine.

[jagerman]

Yeah, it's a property set on the module target set up by pybind11_add_module.

[jagerman]

Is there anything specific that makes my exception class problematic (compared to the stdexcept classes for example), or is this is a generic issue that more people are likely to hit?

If I remember correctly, this doesn't affect stdlibc++ (at least for non-ancient versions), but does affect libc++ (are you on macOS, by any chance? Or on Linux using clang with libc++?). But no, there isn't anything specific to your class: pybind has to deal with the same issue internally for exception class wrappers that we use by registering one exception handler for each module:

pybind11/include/pybind11/detail/internals.h

Lines 146 to 160 in 2d0507d

	// We loaded builtins through python's builtins, which means that our `error_already_set`
	// and `builtin_exception` may be different local classes than the ones set up in the
	// initial exception translator, below, so add another for our local exception classes.
	//
	// libstdc++ doesn't require this (types there are identified only by name)
	#if !defined(__GLIBCXX__)
	(*internals_pp)->registered_exception_translators.push_front(
	[](std::exception_ptr p) -> void {
	try {
	if (p) std::rethrow_exception(p);
	} catch (error_already_set &e) { e.restore(); return;
	} catch (const builtin_exception &e) { e.set_error(); return;
	}
	}
	);

.

[wichert]

I am indeed on macOS.

[bstaletic]

This issue seems resolved.