This is an automated email from the ASF dual-hosted git repository.
njayaram pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/madlib.git
The following commit(s) were added to refs/heads/master by this push:
new ea1e0ac DL: Fix tensorflow crashes
ea1e0ac is described below
commit ea1e0acd26dd970d80f71fcb46ffc831c6cafd81
Author: Domino Valdano <[email protected]>
AuthorDate: Mon Jun 3 14:44:20 2019 -0700
DL: Fix tensorflow crashes
This PR fixes a subset of the problems highlighted in MADLIB-1355.
This commit adds `std::*` to the list of symbols defined in madlib
that should be hidden from external libraries by the linker.
This fixes several intermittent crashes that were happening in the
deep_learning module in different forms on different platforms, due to
tensorflow calling madlib's instantiation of STL template functions,
which circumvents the hiding of madlib's global new and delete symbols
(intended to override the libstdc++ version of these symbols for all
of `libmadlib.so`, but not for anything outside of it).
The main crash we looked at was due to the use of `std::set<string>` in
`madlib/src/modules/linalg/metric.cpp`. Tensorflow is a python library,
but it loads Google's C++ protobuf library `_message.so`. The crash was
happening because `_message.so` also uses `std::set<string>` for some
things. We were able to fix it by hiding all madlib symbols associated
with the `std::set<string` class, but this simply resulted in a
different crash happening instead.
In the long term, we should move to hiding all symbols by default
and explicitly make exceptions for madlib API functions. But this would
be a bigger change and require more extensive testing to make sure it
doesn't cause any obscure issues on any platform. So for now we are just
hiding those in namespace std.
Closes #405
Co-authored-by: Ekta Khanna <[email protected]>
---
src/library.ver | 46 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/src/library.ver b/src/library.ver
index d147b1f..81021d1 100644
--- a/src/library.ver
+++ b/src/library.ver
@@ -1,11 +1,41 @@
VERS_1.0 {
local:
- _Znwm;
- _ZnwmRKSt9nothrow_t;
- _ZdlPv;
- _ZdlPvRKSt9nothrow_t;
- _Znam;
- _ZnamRKSt9nothrow_t;
- _ZdaPv;
- _ZdaPvRKSt9nothrow_t;
+ _Znwm; # operator new(unsigned long);
+ _ZnwmRKSt9nothrow_t; # operator new(unsigned long, std::nothrow_t
const&);
+ _ZdlPv; # operator delete(void*);
+ _ZdlPvRKSt9nothrow_t; # operator delete(void*, std::nothrow_t const&);
+ _Znam; # operator new[](unsigned long);
+ _ZnamRKSt9nothrow_t; # operator new[](unsigned long, std::nothrow_t
const&);
+ _ZdaPv; # operator delete[](void*);
+ _ZdaPvRKSt9nothrow_t; # operator delete[](void*, std::nothrow_t
const&);
+
+ extern "C++" {
+#
+# Hide everything in std namespace (C++ STL symbols):
+#
+# This fixes several intermittent crashes that were happening in the
+# deep_learning module in different forms on different platforms, due to
+# tensorflow calling madlib's instantiation of STL template functions,
+# which circumvents the hiding of madlib's global new and delete symbols
+# (intended to override the libstdc++ version of these symbols for all
+# of libmadlib.so, but not for anything outside of it).
+#
+# The main crash we looked at was due to the use of std::set<string> in
+# madlib/src/modules/linalg/metric.cpp. Tensorflow is a python library,
+# but it loads Google's C++ protobuf library _message.so. The crash was
+# happening because _message.so also uses std::set<string> for some
+# things, and it ends up calling madlib's instantiation of them. We were
+# able to fix it by hiding all madlib symbols associated with the
std::set<string
+# class, but this just resulted in a similar crash happening instead
because of a
+# different STL symbol exposed by madlib.
+#
+# In the long term, we should move to hiding all symbols by default
+# and explicitly make exceptions for madlib API functions. But this would
+# be a bigger change and require more extensive testing to make sure it
+# doesn't cause any obscure issues on any platform. So for now we are just
+# hiding those in namespace std.
+#
+ std::*;
+
+ };
};
