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

Commit

Permalink
[WIP] [BUGFIX] Fix flakey TemporaryDirectory() cleanup on Windows (#2…
Browse files Browse the repository at this point in the history
…1107)

* Trigger CI to show windows-gpu job failures

* Fix TemporaryDirectory cleanup issues on Windows

* Fix PermissionError handling

* Remove temporary comments

* Fix TemporaryDirectory() to yield dir.name

* Remove temp dirs even with exceptions
  • Loading branch information
DickJC123 committed Aug 1, 2022
1 parent db39bb1 commit dedb8c9
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 15 deletions.
13 changes: 5 additions & 8 deletions python/mxnet/gluon/model_zoo/model_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import os
import zipfile
import logging
import tempfile
import uuid
import shutil

from ..utils import download, check_sha1, replace_file
from ..utils import download, check_sha1, replace_file, TemporaryDirectory
from ... import base

_model_sha1 = {name: checksum for checksum, name in [
Expand Down Expand Up @@ -114,11 +112,10 @@ def get_model_file(name, root=os.path.join(base.data_dir(), 'models')):
download(_url_format.format(repo_url=repo_url, file_name=file_name),
path=temp_zip_file_path, overwrite=True)
with zipfile.ZipFile(temp_zip_file_path) as zf:
temp_dir = tempfile.mkdtemp(dir=root)
zf.extractall(temp_dir)
temp_file_path = os.path.join(temp_dir, file_name+'.params')
replace_file(temp_file_path, file_path)
shutil.rmtree(temp_dir)
with TemporaryDirectory(dir=root) as temp_dir:
zf.extractall(temp_dir)
temp_file_path = os.path.join(temp_dir, file_name+'.params')
replace_file(temp_file_path, file_path)
os.remove(temp_zip_file_path)

if check_sha1(file_path, sha1_hash):
Expand Down
2 changes: 1 addition & 1 deletion python/mxnet/gluon/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import numpy as np

from .. import ndarray
from ..util import is_np_shape, is_np_array
from ..util import is_np_shape, is_np_array, TemporaryDirectory
from .. import numpy as _mx_np # pylint: disable=reimported


Expand Down
22 changes: 22 additions & 0 deletions python/mxnet/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import functools
import inspect
import threading
import tempfile
import platform
from contextlib import contextmanager

from struct import calcsize
from .base import (_LIB, check_call, c_str, py_str,
Expand Down Expand Up @@ -1359,3 +1362,22 @@ def dtype_from_number(number):
elif isinstance(number, _np.generic):
return number.dtype
raise TypeError('type {} not supported'.format(str(type(number))))

# This is a wrapping of tempfile.TemporaryDirectory(), known to have cleanup issues on Windows.
# The problem is partially handled as of Python 3.10 by the adding of a 'ignore_cleanup_errors'
# parameter. Once MXNet's Python version is forced to be >= 3.10, a simplification of this
# function to use 'ignore_cleanup_errors' would be possible. Until the fundamental Windows
# issues are resolved, best to use this routine instead of tempfile.TemporaryDirectory().
@contextmanager
def TemporaryDirectory(*args, **kwargs):
"""A context wrapper of tempfile.TemporaryDirectory() that ignores cleanup errors on Windows.
"""
dir = tempfile.TemporaryDirectory(*args, **kwargs)
try:
yield dir.name
finally:
try:
dir.cleanup()
except PermissionError:
if platform.system() != 'Windows':
raise
3 changes: 2 additions & 1 deletion tests/nightly/test_large_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import mxnet as mx

from mxnet.test_utils import rand_ndarray, assert_almost_equal, rand_coord_2d, default_device, check_symbolic_forward, create_2d_tensor
from mxnet.util import TemporaryDirectory
from mxnet import gluon, nd
from common import with_seed
import pytest
Expand Down Expand Up @@ -1028,7 +1029,7 @@ def check_ndarray_convert():

def check_load_save():
x = create_2d_tensor(SMALL_Y, LARGE_X)
with tempfile.TemporaryDirectory() as tmp:
with TemporaryDirectory() as tmp:
tmpfile = os.path.join(tmp, 'large_tensor')
nd.save(tmpfile, [x])
y = nd.load(tmpfile)
Expand Down
4 changes: 2 additions & 2 deletions tests/nightly/test_large_vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@

import os
import sys
import tempfile
import math
import numpy as np
import mxnet as mx

from mxnet.test_utils import rand_ndarray, assert_almost_equal, rand_coord_2d, create_vector
from mxnet.util import TemporaryDirectory
from mxnet import gluon, nd
from common import with_seed
import pytest
Expand Down Expand Up @@ -374,7 +374,7 @@ def check_cast():

def check_load_save():
x = create_vector(size=LARGE_X)
with tempfile.TemporaryDirectory() as tmp:
with TemporaryDirectory() as tmp:
tmpfile = os.path.join(tmp, 'large_vector')
nd.save(tmpfile, [x])
y = nd.load(tmpfile)
Expand Down
2 changes: 1 addition & 1 deletion tests/python/unittest/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import models
from contextlib import contextmanager
import pytest
from tempfile import TemporaryDirectory
from mxnet.util import TemporaryDirectory
import locale

xfail_when_nonstandard_decimal_separator = pytest.mark.xfail(
Expand Down
4 changes: 2 additions & 2 deletions tests/python/unittest/test_deferred_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@

import functools
import operator
import tempfile

import numpy as np

import mxnet as mx
import mxnet._deferred_compute as dc
from mxnet.base import MXNetError
from mxnet.util import TemporaryDirectory
import pytest


Expand Down Expand Up @@ -420,7 +420,7 @@ def _assert_dc_gluon(setup, net, setup_is_deterministic=True, numpy=True, autogr

_all_same(ys_np, ys_hybrid_np)

with tempfile.TemporaryDirectory() as root:
with TemporaryDirectory() as root:
with mx.util.np_shape(True), mx.util.np_array(True):
net.export(root)

Expand Down

0 comments on commit dedb8c9

Please sign in to comment.