Skip to content

Commit

Permalink
SNOW-799408: Close temp file handle before removing temp cache file (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-aling authored Jun 6, 2023
1 parent f2d398a commit 48aa932
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
1 change: 1 addition & 0 deletions DESCRIPTION.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne

- v3.0.5(TBD)
- Added the ability to read Snowflake's central configuration file.
- Improved OCSP response caching to remove tmp cache files on Windows.

- v3.0.4(May 23,2023)
- Fixed a bug in which `cursor.execute()` could modify the argument statement_params dictionary object when executing a multistatement query.
Expand Down
14 changes: 14 additions & 0 deletions src/snowflake/connector/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,12 @@ def _save(self, load_first: bool = True) -> bool:
prefix=fname,
dir=_dir,
)
# tmp_file is an opened OS level handle, which means we need to close it manually.
# https://docs.python.org/3/library/tempfile.html#tempfile.mkstemp
# ideally we shall just use the tmp_file fd to write,
# however, using os.write(tmp_file, bytes) causes seg fault during garbage collection when exiting
# python program.
# thus we fall back to the approach using the normal open() method to open a file and write.
with open(tmp_file, "wb") as w_file:
pickle.dump(self, w_file)
# We write to a tmp file and then move it to have atomic write
Expand All @@ -532,6 +538,14 @@ def _save(self, load_first: bool = True) -> bool:
getmtime(self.file_path),
)
return True
except NameError:
# note: when exiting python program, garbage collection will kick in
# leading to `open` being garbage collected,
# calling `open` raises NameError, we close the tmp file fd here to release the tmp file fd
try:
os.close(tmp_file)
except OSError:
pass
except OSError as o_err:
raise PermissionError(
o_err.errno,
Expand Down
27 changes: 26 additions & 1 deletion test/extras/run.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
#
# Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved.
#

import os
import pathlib
import platform
import subprocess
import sys

import snowflake.connector.ocsp_snowflake

# This script run every Python file in this directory other than this
# one in a subprocess and checks their exit codes

Expand All @@ -23,3 +26,25 @@
]
)
sub_process.check_returncode()
ocsp_cache_dir_path = pathlib.Path(
snowflake.connector.ocsp_snowflake.OCSP_RESPONSE_VALIDATION_CACHE.file_path
).parent
cache_files = set(os.listdir(ocsp_cache_dir_path))
# This is to test SNOW-79940, making sure tmp files are removed
# Windows does not have ocsp_response_validation_cache.lock
assert (
cache_files
== {
"ocsp_response_validation_cache.lock",
"ocsp_response_validation_cache",
"ocsp_response_cache.json",
}
and not platform.system() == "Windows"
) or (
cache_files
== {
"ocsp_response_validation_cache",
"ocsp_response_cache.json",
}
and platform.system() == "Windows"
)

0 comments on commit 48aa932

Please sign in to comment.