box / box-python-sdk

Box SDK for Python
http://opensource.box.com/box-python-sdk/
Apache License 2.0
419 stars 215 forks source link

ChunkedUploader Resume Not Closing ThreadPoolExecutor Instances #839

Closed Badgeoak closed 1 year ago

Badgeoak commented 1 year ago

Description of the Issue

Hi team, long time user, first time issue-opener. Sorry if I missed anything obvious!

The ChunkedUploader's resume method doesn't shut down the ThreadPoolExecutor after it successfully uploads.

I am uploading a few files of a few hundred MB, get a ConnectionError, and successfully call the chunked_uploader.resume() and it completes, but I still see two instances of the ThreadPoolExecutor running. I modified the code in the resumse method as below to double check what I was seeing.

self._executor = ThreadPoolExecutor(max_workers=API.CHUNK_UPLOAD_THREADS, thread_name_prefix='box_resume') And even after the files are uploaded I have box_resume_0 and box_resume_1 in my call stack.

Steps to Reproduce

Start a chunked_uploader on a large enough file Get a ConnectionError Call chunked_uploader.resume()

Expected Behavior

Expect to see threadpool get cleaned up

Versions Used

Python SDK: 3.8.1 Python: 3.9.6

lukaszsocha2 commented 1 year ago

Hi @Badgeoak, thanks for posting this issue. Can you check if pasting this line: self._executor.shutdown(wait=True) to line 156 in chunked_uploader.py (just before return statement in _commit_and_erase_stream_reference_when_succeed method) solves your issue ? Best, @lukaszsocha2

Badgeoak commented 1 year ago

@lukaszsocha2 Yep, that seems to do the trick!

lukaszsocha2 commented 1 year ago

We will do the fix then, thanks. Best, @lukaszsocha2