Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[v1.9.x] stop closing opened libs #20523

Merged
merged 3 commits into from
Aug 18, 2021
Merged

[v1.9.x] stop closing opened libs #20523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbbb290
stopped closing opened libs
Aug 12, 2021
03ce69d
removed return handling from load
Aug 18, 2021
fdffbe8
missed one *lib
Aug 18, 2021
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: 3 additions & 2 deletions include/mxnet/c_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,11 @@ MXNET_DLL const char *MXGetLastError();
/*!
* \brief Load library dynamically
* \param path to the library .so file
* \param 0 for quiet, 1 for verbose
* \param verbose 0 for quiet, 1 for verbose
* \param lib handle to opened library
* \return 0 when success, -1 when failure happens.
*/
MXNET_DLL int MXLoadLib(const char *path, unsigned verbose);
MXNET_DLL int MXLoadLib(const char *path, unsigned verbose, void** lib);
Copy link
Member

Choose a reason for hiding this comment

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

This is a C API change. Is it required for the fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @szha! No this change isnt required, but without it we cannot return a handle to the opened library. Its not required per-se since opening the same library using ctypes or some other API will return the same handle anyway (with linux doing the mapping). So we can remove this change if that is important, either way is fine to me.

Copy link
Member

Choose a reason for hiding this comment

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

C API is part of the API to maintain semantic versioning for. Let's not include the API change then.


/*!
* \brief Get list of features supported on the runtime
Expand Down
7 changes: 5 additions & 2 deletions python/mxnet/library.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def load(path, verbose=True):

Returns
---------
void
ctypes.c_void_p : handle to opened library
"""
#check if path exists
if not os.path.exists(path):
Expand All @@ -53,7 +53,8 @@ def load(path, verbose=True):
verbose_val = 1 if verbose else 0
byt_obj = path.encode('utf-8')
chararr = ctypes.c_char_p(byt_obj)
check_call(_LIB.MXLoadLib(chararr, mx_uint(verbose_val)))
lib_ptr = ctypes.c_void_p(0)
check_call(_LIB.MXLoadLib(chararr, mx_uint(verbose_val), ctypes.byref(lib_ptr)))

#regenerate operators
_init_op_module('mxnet', 'ndarray', _make_ndarray_function)
Expand All @@ -72,3 +73,5 @@ def load(path, verbose=True):
for op in dir(mx_sym_op):
func = getattr(mx_sym_op, op)
setattr(mx_sym, op, func)

return lib_ptr
23 changes: 13 additions & 10 deletions src/c_api/c_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1514,39 +1514,42 @@ void registerPasses(void *lib, int verbose, mxnet::ext::msgSize_t msgSize,
/*!
* \brief Loads dynamic custom library and initializes it
* \param path library path
* \param verbose 0 for quiet, 1 for verbose
* \param lib handle to opened library
* \return 0 when success, -1 when failure happens.
*/
int MXLoadLib(const char *path, unsigned verbose) {
int MXLoadLib(const char *path, unsigned verbose, void** lib) {
API_BEGIN();
void *lib = LibraryInitializer::Get()->lib_load(path);
if (!lib)
*lib = LibraryInitializer::Get()->lib_load(path);
if (!*lib)
LOG(FATAL) << "Unable to load library";

// check that library and MXNet use same version of library API
mxnet::ext::opVersion_t opVersion =
get_func<mxnet::ext::opVersion_t>(lib, const_cast<char*>(MXLIB_OPVERSION_STR));
get_func<mxnet::ext::opVersion_t>(*lib, const_cast<char*>(MXLIB_OPVERSION_STR));
int libVersion = opVersion();
if (MX_LIBRARY_VERSION != libVersion)
LOG(FATAL) << "Library version (" << libVersion << ") does not match MXNet version ("
<< MX_LIBRARY_VERSION << ")";

// get error messaging APIs
mxnet::ext::msgSize_t msgSize =
get_func<mxnet::ext::msgSize_t>(lib, const_cast<char*>(MXLIB_MSGSIZE_STR));
get_func<mxnet::ext::msgSize_t>(*lib, const_cast<char*>(MXLIB_MSGSIZE_STR));
mxnet::ext::msgGet_t msgGet =
get_func<mxnet::ext::msgGet_t>(lib, const_cast<char*>(MXLIB_MSGGET_STR));
get_func<mxnet::ext::msgGet_t>(*lib, const_cast<char*>(MXLIB_MSGGET_STR));

// initialize library by passing MXNet version
mxnet::ext::initialize_t initialize =
get_func<mxnet::ext::initialize_t>(lib, const_cast<char*>(MXLIB_INITIALIZE_STR));
get_func<mxnet::ext::initialize_t>(*lib, const_cast<char*>(MXLIB_INITIALIZE_STR));
if (!initialize(static_cast<int>(MXNET_VERSION))) {
std::string msgs = getExtensionMsgs(msgSize, msgGet);
LOG(FATAL) << "Library failed to initialize" << msgs;
}

// find ops, partitioners, and passes in library
registerOperators(lib, verbose, msgSize, msgGet);
registerPartitioners(lib, verbose, msgSize, msgGet);
registerPasses(lib, verbose, msgSize, msgGet);
registerOperators(*lib, verbose, msgSize, msgGet);
registerPartitioners(*lib, verbose, msgSize, msgGet);
registerPasses(*lib, verbose, msgSize, msgGet);
API_END();
}

Expand Down
4 changes: 0 additions & 4 deletions src/initialize.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ LibraryInitializer::LibraryInitializer()
install_pthread_atfork_handlers();
}

LibraryInitializer::~LibraryInitializer() {
close_open_libs();
}

bool LibraryInitializer::lib_is_loaded(const std::string& path) const {
return loaded_libs.count(path) > 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/initialize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class LibraryInitializer {
*/
LibraryInitializer();

~LibraryInitializer();
~LibraryInitializer() = default;

/**
* @return true if the current pid doesn't match the one that initialized the library
Expand Down