Bug in convert_os_errors

[isaacHuh]

https://github.com/PyFilesystem/pyfilesystem2/blob/2d0ffc34a2127db3fdf0f8fef6f4644bf7500a1e/fs/error_tools.py#L87

This compares a tuple to int and is always false.

[dargueta]

PR: https://github.com/PyFilesystem/pyfilesystem2/pull/491

[lurch]

@isaacHuh In #491 we're trying to figure out what this code is supposed to be doing... do you have a reliable way of triggering this exception? :slightly_smiling_face:

[isaacHuh]

@isaacHuh In #491 we're trying to figure out what this code is supposed to be doing... do you have a reliable way of triggering this exception? 🙂

@lurch So I would do this to test:

from fs.error_tools import convert_os_errors
import errno

with convert_os_errors("test", "path"):
    o = OSError(32)
    o.errno = errno.EACCES
    raise o

should be resource locked error but I just found I get this:

fs.errors.FileExpected: path 'path' should be a file

https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/error_tools.py#L63

this line prevents the correct error raising since it remaps FILE_ERRORS[13] and errno.EACCES is 13 so _errno == errno.EACCES is always false.

[lurch]

@isaacHuh I meant a way to provoke this from actual pyfilesystem code, not provoking it by manually constructing an "artificial OSError" that we may or may not ever actually see in the real world :wink:

[isaacHuh]

@lurch

from fs.osfs import OSFS
import os
import win32file
import win32con
import pywintypes
from fs.error_tools import convert_os_errors

f = OSFS('./')
path = "test_file.story"
f.create(path)
sys_path = f.getsyspath(path)

fd = os.open(sys_path, os.O_RDWR)

win32file.LockFileEx(
    win32file._get_osfhandle(fd),
    (win32con.LOCKFILE_FAIL_IMMEDIATELY | win32con.LOCKFILE_EXCLUSIVE_LOCK),
    0, -0x10000, pywintypes.OVERLAPPED())

# do this in a new process or something:
new_content = b'123456'
with convert_os_errors("test", sys_path):
    os.write(fd, new_content)

win32file.UnlockFileEx(win32file._get_osfhandle(fd), 0, -0x10000, pywintypes.OVERLAPPED())
os.close(fd)

haven't gotten this working but something like this and you could try creating another process that interacts with the file while there is a lock on it. Or you could use an application like storyline 360 that holds a lock on a file to test and have that file open while you run a script that attempts to write to it. Both should work but haven't tried it myself.

idk if thats what you were looking for

[lurch]

idk if thats what you were looking for

Yes, that's more along the lines of what I meant with "do you have a reliable way of triggering this exception?" :+1:

haven't gotten this working

Ahh, that's a shame. As I suggested in #491 "I guess there's a possibility that this code may not even be needed now, given the changes to both Windows and Python over the past 12 years?" :man_shrugging: :man_shrugging:

[isaacHuh]

@lurch oh ok I see. This code does get hit when a process is attempting to read or write to a file that another process has a lock on it tho. I've seen storyline 360 causing this to get hit because that application holds a lock on a file the entire time it has the file open while most other applications don't, so any other process attempting to read from the file throws would throw a resource locked error.

I played with it a bit after work and this raises the exception. just run file 1 and run file 2 while file 1 is sleep. Again I get fs.errors.FileExpected when it should be resource locked when I run this because of the bug but this is it:

file 1:

from fs.error_tools import convert_os_errors
from multiprocessing import Process
import time

f = OSFS('./')
path = "test_file.story"
f.create(path)
sys_path = f.getsyspath(path)

fd = os.open(sys_path, os.O_RDWR)

win32file.LockFileEx(
    win32file._get_osfhandle(fd),
    (win32con.LOCKFILE_FAIL_IMMEDIATELY | win32con.LOCKFILE_EXCLUSIVE_LOCK),
    0, -0x10000, pywintypes.OVERLAPPED())

time.sleep(30)

win32file.UnlockFileEx(win32file._get_osfhandle(fd), 0, -0x10000, pywintypes.OVERLAPPED())
os.close(fd)

file 2:

from fs.error_tools import convert_os_errors
import os

path = "test_file.story"
sys_path = path

fd = os.open(sys_path, os.O_RDWR)

new_content = b'123456'
with convert_os_errors("test", sys_path):
    os.write(fd, new_content)

[isaacHuh]

ok this works too:

from fs.osfs import OSFS
import os
import win32file
import win32con
import pywintypes
from fs.error_tools import convert_os_errors
from multiprocessing import Process
import time

def ex_other():
    #exec(open("C:\\Users\\Isaac Torres\\test.py").read())
    f = OSFS('./')
    path = "test_file.story"
    sys_path = f.getsyspath(path)
    fd = os.open(path, os.O_RDWR)

    win32file.LockFileEx(
        win32file._get_osfhandle(fd),
        (win32con.LOCKFILE_FAIL_IMMEDIATELY | win32con.LOCKFILE_EXCLUSIVE_LOCK),
        0, -0x10000, pywintypes.OVERLAPPED())

    #exec(open("test2.py").read())
    time.sleep(30)

    win32file.UnlockFileEx(win32file._get_osfhandle(fd), 0, -0x10000, pywintypes.OVERLAPPED())
    os.close(fd)

if __name__ == '__main__':
    f = OSFS('./')
    path = "test_file.story"
    f.create(path)
    sys_path = f.getsyspath(path)

    p = Process(target=ex_other)
    p.start()
    time.sleep(1)

    fd = os.open(sys_path, os.O_RDWR)

    new_content = b'123456'
    with convert_os_errors("test", sys_path):
        os.write(fd, new_content)

    p.join()

[lurch]

Fantastic, thanks! :+1: ping @dargueta I'll try to have a fiddle with this myself tomorrow evening, unless someone beats me to it.

[dargueta]

I don't have access to a Windows machine to test this, but could you change

with convert_os_errors("test", sys_path):
    os.write(fd, new_content)

to

import pprint
try:
    with convert_os_errors("test", sys_path):
        os.write(fd, new_content)
except Exception as err:
    pprint.pprint(vars(err))
    raise

and see what that spits out? It won't fix anything, it'll just give me a bit more to work with.

[dargueta]

@isaacHuh we're still having trouble getting this to work properly, can you help us out in #491?

[isaacHuh]

@dargueta yeah I'll take a look. Here's the previous output from before:

{'_msg': "path '{path}' should be a file",
 'exc': PermissionError(13, 'Permission denied'),
 'path': 'C:\\Users\\Isaac Torres\\Desktop\\test_file.story'}
Traceback (most recent call last):
  File "C:\Users\Isaac Torres\Desktop\test2.py", line 46, in <module>
    os.write(fd, new_content)
PermissionError: [Errno 13] Permission denied

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\Isaac Torres\Desktop\test2.py", line 46, in <module>
    os.write(fd, new_content)
  File "C:\Users\Isaac Torres\pyenv\lib\site-packages\fs\error_tools.py", line 89, in __exit__
    reraise(fserror, fserror(self._path, exc=exc_value), traceback)
  File "C:\Users\Isaac Torres\pyenv\lib\site-packages\six.py", line 718, in reraise
    raise value.with_traceback(tb)
  File "C:\Users\Isaac Torres\Desktop\test2.py", line 46, in <module>
    os.write(fd, new_content)
fs.errors.FileExpected: path 'C:\Users\Isaac Torres\Desktop\test_file.story' should be a file

I didn't see your previous message before