You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/09/27 07:10:27 UTC
[GitHub] [incubator-mxnet] yajiedesign opened a new pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
yajiedesign opened a new pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236
## Description ##
fix python 3.8 ctypes dll load with windows
## Checklist ##
### Essentials ###
- [X] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
- [X] Changes are complete (i.e. I finished coding on this PR)
### Changes ###
## Comments ##
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on a change in pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on a change in pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#discussion_r497187645
##########
File path: python/mxnet/base.py
##########
@@ -276,7 +276,12 @@ class MXCallbackList(ctypes.Structure):
def _load_lib():
"""Load library by searching possible path."""
lib_path = libinfo.find_lib_path()
- lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
+ if sys.version_info >= (3, 8) and os.name == "nt":
+ # use LOAD_WITH_ALTERED_SEARCH_PATH, For simplicity, let's just fill the numbers.
+ # pylint: disable=E1123
+ lib = ctypes.CDLL(lib_path[0], winmode=0x00000008)
Review comment:
Not at the moment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-699713533
run ci [windows-gpu]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha merged pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
szha merged pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] wkcn commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
wkcn commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-700357696
https://docs.python.org/3/library/ctypes.html
The document shows that
> The winmode parameter is used on Windows to specify how the library is loaded (since mode is ignored). It takes any value that is valid for the Win32 API LoadLibraryEx flags parameter. When omitted, the default is to use the flags that result in the most secure DLL load to avoiding issues such as DLL hijacking. Passing the full path to the DLL is the safest way to ensure the correct library and dependencies are loaded.
Could we pass the full path of DLL by ` lib = ctypes.CDLL(os.path.abspath(lib_path[0]))` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-699597224
@szha @aaronmarkham Lint doesn't know the new parameter. What should I do?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-699596205
Hey @yajiedesign , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [windows-cpu, windows-gpu, unix-cpu, sanity, website, centos-cpu, unix-gpu, clang, edge, centos-gpu, miscellaneous]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-700716136
@wkcn The key is to find CUDA's dll. Adjusting mxnet.dll path doesn't help.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on a change in pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on a change in pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#discussion_r497187645
##########
File path: python/mxnet/base.py
##########
@@ -276,7 +276,12 @@ class MXCallbackList(ctypes.Structure):
def _load_lib():
"""Load library by searching possible path."""
lib_path = libinfo.find_lib_path()
- lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
+ if sys.version_info >= (3, 8) and os.name == "nt":
+ # use LOAD_WITH_ALTERED_SEARCH_PATH, For simplicity, let's just fill the numbers.
+ # pylint: disable=E1123
+ lib = ctypes.CDLL(lib_path[0], winmode=0x00000008)
Review comment:
Not at the moment.So I added some comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on a change in pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
szha commented on a change in pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#discussion_r495607149
##########
File path: python/mxnet/base.py
##########
@@ -276,7 +276,15 @@ class MXCallbackList(ctypes.Structure):
def _load_lib():
"""Load library by searching possible path."""
lib_path = libinfo.find_lib_path()
- lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
+ if sys.version_info >= (3, 8):
+ if os.name == "nt":
+ # use LOAD_WITH_ALTERED_SEARCH_PATH, For simplicity, let's just fill the numbers.
+ # pylint: disable=E1123
+ lib = ctypes.CDLL(lib_path[0], winmode=0x00000008)
+ else:
+ lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
+ else:
+ lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
Review comment:
nit:
```suggestion
if sys.version_info >= (3, 8) and os.name == "nt":
# use LOAD_WITH_ALTERED_SEARCH_PATH, For simplicity, let's just fill the numbers.
# pylint: disable=E1123
lib = ctypes.CDLL(lib_path[0], winmode=0x00000008)
else:
lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] marcoabreu commented on a change in pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
marcoabreu commented on a change in pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#discussion_r497092091
##########
File path: python/mxnet/base.py
##########
@@ -276,7 +276,12 @@ class MXCallbackList(ctypes.Structure):
def _load_lib():
"""Load library by searching possible path."""
lib_path = libinfo.find_lib_path()
- lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_LOCAL)
+ if sys.version_info >= (3, 8) and os.name == "nt":
+ # use LOAD_WITH_ALTERED_SEARCH_PATH, For simplicity, let's just fill the numbers.
+ # pylint: disable=E1123
+ lib = ctypes.CDLL(lib_path[0], winmode=0x00000008)
Review comment:
Is the magic value available as a constant somewhere?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] yajiedesign commented on pull request #19236: [BUGFIX]fix python 3.8 ctypes dll load with windows
Posted by GitBox <gi...@apache.org>.
yajiedesign commented on pull request #19236:
URL: https://github.com/apache/incubator-mxnet/pull/19236#issuecomment-699598553
I disable it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org