#34294 closed Bug (fixed)
File locking fails if argtypes redefined on Windows.
Reported by: | Simon Sawicki | Owned by: | Simon Sawicki |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
django.core.files.locks
uses ctypes.windll.kernel32
which is a global instance. If other code redefines windll.kernel32.LockFileEx.argtypes
the function fails with an error.
import ctypes import ctypes.wintypes from django.core.files import locks class OVERLAPPED(ctypes.Structure): _fields_ = [ ("Internal", ctypes.wintypes.LPVOID), ("InternalHigh", ctypes.wintypes.LPVOID), ("Offset", ctypes.wintypes.DWORD), ("OffsetHigh", ctypes.wintypes.DWORD), ("hEvent", ctypes.wintypes.HANDLE), ] ctypes.windll.kernel32.LockFileEx.argtypes = [ ctypes.wintypes.HANDLE, ctypes.wintypes.DWORD, ctypes.wintypes.DWORD, ctypes.wintypes.DWORD, ctypes.wintypes.DWORD, ctypes.wintypes.DWORD, ] with open("./file", "w") as f: locks.lock(f, locks.LOCK_EX)
Possible patch:
-
django/core/files/locks.py
diff --git a/django/core/files/locks.py b/django/core/files/locks.py index 5752b08a69..c0f471f87d 100644
a b if os.name == "nt": 32 32 POINTER, 33 33 Structure, 34 34 Union, 35 WinDLL, 35 36 byref, 36 37 c_int64, 37 38 c_ulong, 38 39 c_void_p, 39 40 sizeof, 40 windll,41 41 ) 42 42 from ctypes.wintypes import BOOL, DWORD, HANDLE 43 43 … … if os.name == "nt": 73 73 LPOVERLAPPED = POINTER(OVERLAPPED) 74 74 75 75 # --- Define function prototypes for extra safety --- 76 LockFileEx = windll.kernel32.LockFileEx 76 kernel32 = WinDLL("kernel32") 77 LockFileEx = kernel32.LockFileEx 77 78 LockFileEx.restype = BOOL 78 79 LockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, DWORD, LPOVERLAPPED] 79 UnlockFileEx = windll.kernel32.UnlockFileEx80 UnlockFileEx = kernel32.UnlockFileEx 80 81 UnlockFileEx.restype = BOOL 81 82 UnlockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, LPOVERLAPPED] 82 83 -
tests/files/tests.py
diff --git a/tests/files/tests.py b/tests/files/tests.py index b3478d2732..d0e633cb7a 100644
a b class FileTests(unittest.TestCase): 200 200 self.assertIs(locks.unlock(f1), True) 201 201 self.assertIs(locks.unlock(f2), True) 202 202 203 @unittest.skipIf(os.name != "nt", "File locking usees WinDLL on windows only") 204 def test_local_windll(self): 205 import ctypes 206 import ctypes.wintypes 207 208 old_argtypes = ctypes.windll.kernel32.LockFileEx.argtypes 209 try: 210 ctypes.windll.kernel32.LockFileEx.argtypes = [ 211 ctypes.wintypes.HANDLE, 212 ctypes.wintypes.DWORD, 213 ctypes.wintypes.DWORD, 214 ctypes.wintypes.DWORD, 215 ctypes.wintypes.DWORD, 216 ctypes.wintypes.DWORD, 217 ] 218 file_path = Path(__file__).parent / "test.png" 219 with open(file_path) as file: 220 try: 221 locks.lock(file, locks.LOCK_EX) 222 except Exception: 223 self.fail("Locking raised exception with changed argtypes") 224 else: 225 locks.unlock(file) 226 finally: 227 ctypes.windll.kernel32.LockFileEx.argtypes = old_argtypes 228 203 229 204 230 class NoNameFileTestCase(unittest.TestCase): 205 231 """
Change History (8)
comment:1 by , 2 years ago
Description: | modified (diff) |
---|
comment:2 by , 2 years ago
Component: | Core (Other) → File uploads/storage |
---|---|
Has patch: | unset |
Owner: | changed from | to
Status: | new → assigned |
Summary: | File locking fails if argtypes redefined → File locking fails if argtypes redefined on Windows. |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:3 by , 2 years ago
This caused https://github.com/ytdl-org/youtube-dl/issues/21545 for example, since yt-dl (possibly also other projects) also mutates that global instance. I will open a PR asap, thank you for the quick triage!
comment:5 by , 2 years ago
For historic cross-referencing purposes, the aforementioned youtube-dl issue ultimately ended up tracked as #30603.
comment:7 by , 2 years ago
Type: | Cleanup/optimization → Bug |
---|
Note:
See TracTickets
for help on using tickets.
Thanks for this ticket.
locks
is an undocumented internal API, but I agree it's worth improving. At the same time, I'm not sure if a pathological and rare case as this one is worth extra tests. I'd accept PR without tests.