Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in convert_os_errors #484

Open
isaacHuh opened this issue Jun 1, 2021 · 12 comments · May be fixed by #491
Open

Bug in convert_os_errors #484

isaacHuh opened this issue Jun 1, 2021 · 12 comments · May be fixed by #491
Labels

Comments

@isaacHuh
Copy link

isaacHuh commented Jun 1, 2021

if getattr(exc_value, "args", None) == 32: # pragma: no cover

This compares a tuple to int and is always false.

@althonos althonos added the bug label Jun 27, 2021
dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
@dargueta dargueta linked a pull request Jul 29, 2021 that will close this issue
4 tasks
@dargueta
Copy link
Contributor

PR: #491

dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
dargueta pushed a commit to dargueta/pyfilesystem2 that referenced this issue Jul 29, 2021
@lurch
Copy link
Contributor

lurch commented Aug 3, 2021

@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? 🙂

@isaacHuh
Copy link
Author

isaacHuh commented Aug 3, 2021

@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
Copy link
Contributor

lurch commented Aug 3, 2021

@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 😉

@isaacHuh
Copy link
Author

isaacHuh commented Aug 3, 2021

@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
Copy link
Contributor

lurch commented Aug 3, 2021

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?" 👍

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?" 🤷‍♂️ 🤷‍♂️

@isaacHuh
Copy link
Author

isaacHuh commented Aug 4, 2021

@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
Copy link
Author

isaacHuh commented Aug 4, 2021

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
Copy link
Contributor

lurch commented Aug 4, 2021

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

@dargueta
Copy link
Contributor

dargueta commented Aug 4, 2021

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
Copy link
Contributor

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

@isaacHuh
Copy link
Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants