Skip to content

Avoid global multiprocessing locks#617

Merged
casperdcl merged 2 commits intotqdm:masterfrom
mjstevens777:fix/multiprocessing-lock
Jan 6, 2019
Merged

Avoid global multiprocessing locks#617
casperdcl merged 2 commits intotqdm:masterfrom
mjstevens777:fix/multiprocessing-lock

Conversation

@mjstevens777
Copy link
Copy Markdown
Contributor

@mjstevens777 mjstevens777 commented Sep 21, 2018

  • Moving locks to class properties
  • Adding lazy initialization for multiprocessing locks

Fixes #611

The current behavior of initializing a multiprocessing lock is undocumented behavior. The main documentation says to feed in a lock explicitly for windows compatibility.

  • I have visited the source website, and in particular
    read the known issues
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and
    environment, where applicable: 
    import tqdm, sys
print(tqdm.__version__, sys.version, sys.platform)
  • If applicable, I have mentioned the relevant/related issue(s)

Here is how this change affects the following use cases:

  • Create a multiprocessing pool, then use tqdm to wrap pool.imap or similar methods.
    • Write locking will no longer work, since the lock will not exist at the time of forking
  • Same as above, but create the progress bar before the pool
    • Write locking will be unaffected since the progress bar will initialize the write lock before forking
  • Set the multiprocessing method to spawn, create a progress bar in the main thread, run a multiprocessing pool
  • Create a thread pool, create a set of progress bars inside of the threads
    • This should be unaffected
  • Feed in the lock explicitly with tqdm.set_lock
    • This should be unaffected

@chengs
Copy link
Copy Markdown
Contributor

chengs commented Sep 22, 2018 •
edited
Loading

@casperdcl I think now it's the time to remove py26 from travis.ci. Anyway, it is no longer supported officially and it raises an compatibility error..

@chengs
Copy link
Copy Markdown
Contributor

chengs commented Sep 22, 2018

IHMO, maybe it's better to have a testcase for #611.

@casperdcl casperdcl self-assigned this Sep 22, 2018
@casperdcl casperdcl added p0-bug-critical ☢ Exception rasing p3-enhancement 🔥 Much new such feature synchronisation ⇶ Multi-thread/processing to-review 🔍 Awaiting final confirmation labels Sep 22, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 17, 2018 •
edited
Loading

Codecov Report

Merging #617 into master will increase coverage by 0.14%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master     #617      +/-   ##
==========================================
+ Coverage   98.78%   98.93%   +0.14%     
==========================================
  Files          10       10              
  Lines         743      750       +7     
  Branches      132      134       +2     
==========================================
+ Hits          734      742       +8     
+ Misses          5        4       -1     
  Partials        4        4

@mjstevens777
Copy link
Copy Markdown
Contributor Author

mjstevens777 commented Oct 17, 2018

Sorry for the long delay. I have added a test. Python2.7 is failing because of a bug in requests/coveralls, but otherwise the tests are passing. Please let me know if there are any other changes you would like me to make.

@casperdcl
Copy link
Copy Markdown
Member

casperdcl commented Oct 17, 2018

does it fix #611 ? Sorry don't have much time to look into everything.

@mjstevens777
Copy link
Copy Markdown
Contributor Author

mjstevens777 commented Oct 18, 2018

Yeah, it fixes the issue. Creating the multiprocessing.RLock when importing tqdm was what caused the error, and that no longer happens.

@chengs
Copy link
Copy Markdown
Contributor

chengs commented Nov 8, 2018

@casperdcl I think this PL is important and worth being merged into master ASAP.

@casperdcl
Copy link
Copy Markdown
Member

casperdcl commented Nov 10, 2018

yes been über busy, will take a look soon hopefully

@chengs
Copy link
Copy Markdown
Contributor

chengs commented Dec 16, 2018

ping

@chengs chengs mentioned this pull request Dec 16, 2018
Open
@mjstevens777 @casperdcl
Avoid global multiprocessing locks
b566af8 
- Moving locks to class properties
- Adding lazy initialization for multiprocessing locks
@mjstevens777 @casperdcl
Adding multiprocessing test
51b5e6f
@casperdcl casperdcl added to-merge ↰ Imminent and removed to-review 🔍 Awaiting final confirmation labels Jan 6, 2019
@casperdcl casperdcl merged commit 51b5e6f into tqdm:master Jan 6, 2019
@mjstevens777 mjstevens777 mentioned this pull request Jan 11, 2019
Merged
casperdcl added a commit that referenced this pull request Jan 11, 2019
@casperdcl
bump version, merge branch 'devel'
e3fcc40 
- fixes #457, closes #658 caused by #617
@neerajprad neerajprad mentioned this pull request May 1, 2019
Merged
@casperdcl casperdcl mentioned this pull request Jun 24, 2019
Merged
@mparry mparry mentioned this pull request Jul 29, 2020
Merged
This was referenced Oct 6, 2020
Merged
Open
Merged
thewtex added a commit to thewtex/ITK that referenced this pull request May 25, 2021
@thewtex
BUG: Do not create global multiprocessing RLock
471ca04 
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
@thewtex thewtex mentioned this pull request May 25, 2021
Merged
thewtex added a commit to thewtex/ITK that referenced this pull request May 25, 2021
@thewtex
BUG: Do not create global multiprocessing RLock
1b84f86 
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
thewtex added a commit to thewtex/ITK that referenced this pull request May 25, 2021
@thewtex
BUG: Do not create global multiprocessing RLock
4abc72b 
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
thewtex added a commit to thewtex/ITK that referenced this pull request May 25, 2021
@thewtex
BUG: Do not create global multiprocessing RLock
1d4b75c 
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.
thewtex added a commit to thewtex/ITK that referenced this pull request May 26, 2021
@thewtex
BUG: Do not create global multiprocessing RLock
cfdb502 
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.
@jeromekelleher jeromekelleher mentioned this pull request May 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@casperdcl casperdcl

Labels

p0-bug-critical ☢ Exception rasing p3-enhancement 🔥 Much new such feature synchronisation ⇶ Multi-thread/processing to-merge ↰ Imminent

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mjstevens777 @chengs @codecov-io @casperdcl