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

Avoid modifying loaded library map while iterating in lib_close() #20941

Merged
merged 3 commits into from
Mar 8, 2022
Merged

Avoid modifying loaded library map while iterating in lib_close() #20941

merged 3 commits into from
Mar 8, 2022

Conversation

josephevans
Copy link
Contributor

@josephevans josephevans commented Mar 8, 2022 •
edited

Update close libs to not modify map while iterating over opened libraries, rename loaded_libs to loaded_libs_ to signify it is a private member.

This fixes a sporadic crash when closing MXNet (observed with MXNet and Neuron):

terminate called after throwing an instance of 'dmlc::Error'
  what():  [06:24:16] ../src/initialize.cc:154: Check failed: !libpath.empty():

@josephevans
Update close libs to not modify map while iterating over opened libra…
ebf39fb 
…ries, rename loaded_libs to loaded_libs_ to signify it is a private member.
@mxnet-bot
Copy link

Hey @josephevans , 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: [edge, sanity, website, unix-gpu, centos-cpu, windows-gpu, clang, unix-cpu, centos-gpu, miscellaneous, windows-cpu]

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.

@mseth10 mseth10 added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Mar 8, 2022
@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Mar 8, 2022
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Mar 8, 2022
waytrue17
waytrue17 approved these changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@waytrue17 waytrue17 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress labels Mar 8, 2022
@josephevans josephevans merged commit 13b8690 into apache:master Mar 8, 2022
@josephevans josephevans deleted the fix_close_libs branch March 8, 2022 17:46
josephevans added a commit to josephevans/mxnet that referenced this pull request Mar 8, 2022
@josephevans
Avoid modifying loaded library map while iterating in lib_close() (ap…
718dbc4 
…ache#20941)

* Update close libs to not modify map while iterating over opened libraries, rename loaded_libs to loaded_libs_ to signify it is a private member.

* Clean up and simplify library close code.

* Fix clang-format.
josephevans added a commit to josephevans/mxnet that referenced this pull request Mar 8, 2022
@josephevans
Avoid modifying loaded library map while iterating in lib_close() (ap…
17b913f 
…ache#20941)

* Update close libs to not modify map while iterating over opened libraries, rename loaded_libs to loaded_libs_ to signify it is a private member.

* Clean up and simplify library close code.

* Fix clang-format.
This was referenced Mar 8, 2022
Merged
Merged
josephevans added a commit that referenced this pull request Mar 9, 2022
@josephevans
Avoid modifying loaded library map while iterating in lib_close() (#2…
63602d7 
…0941) (#20943)

* Update close libs to not modify map while iterating over opened libraries, rename loaded_libs to loaded_libs_ to signify it is a private member.

* Clean up and simplify library close code.

* Fix clang-format.
josephevans added a commit that referenced this pull request Mar 9, 2022
@josephevans
Avoid modifying loaded library map while iterating in lib_close() (#2…
32cef8e 
…0941) (#20944)

* Update close libs to not modify map while iterating over opened libraries, rename loaded_libs to loaded_libs_ to signify it is a private member.

* Clean up and simplify library close code.

* Fix clang-format.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@szha szha szha approved these changes

@waytrue17 waytrue17 waytrue17 approved these changes

@barry-jin barry-jin Awaiting requested review from barry-jin

Assignees
No one assigned
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@josephevans @mxnet-bot @szha @waytrue17 @mseth10