Multiplatform locking

[Xorboo]

In regards to issue #14

1. Using msvcrt.locking to lock a file on windows (same call is used in fasteners). However, it requires some logic changes, because reading/writing to a locked file from a second handler seems to be impossible on windows and throws PermissionError[13] (while opening still works). I had to:

   • Disable some tests that try to read/write to locked file directly. This shouldn't be used in a normal program anyway.
   • Raise a new windows-only exception on allow_samepid flag, as otherwise second pidfile will try to read a file and raise an exception.
   • Manually try to read from a file when opening it to check if file is locked.

2. Using psutil for checking if process is running. os.kill() doesn't work on widows, so we have to change it as well. Again, same call on POSIX, custom one on windows

3. Ignoring some commands on windows, as they are only available on Unix:

   • os.fchmod()
   • os.fchown()
   • os.getgid()

All tests run smoothly and decorator seems to be working fine in my project.

Notes:

• I'm not sure if I changed setup.py correctly, needs checking.
• Running two scripts at exactly the same time doesn't stop any of them, is there is the same racing condition on Linux?
• idna (required by coverage) does not support python2.6 anymore, so i removed 2.6 support
• psutil does not support python3.3, removed it as well.
• Is there a way to somehow cover windows-only part of the code with tests?

[coveralls]

Coverage decreased (-0.1%) to 94.505% when pulling d1d294474985c0f0b30cdcb9e1c65cc95f30db9f on Xorboo:feature/windows-locking into 0efff53af4554dfc8e06a2627809baa84d13b732 on trbs:master.

[trbs]

Thanks for all your work on this !

There is a travis beta for windows builds... https://blog.travis-ci.com/2018-10-11-windows-early-release but I have not used it.

Btw I like psutil but we might want to consider not using it and copying over their implementation of pid_exists such that we do not an external dependency.

[Xorboo]

Hey there, took me a while to get my hands on it:

1. I added windows platform to Travis, python is not officially supported, but we can install it via shell commands, seems to be working well! Except for python3.4, for which psutil seems to require Microsoft Visual C++ 10.0 for some reason, resulting in a fail build.

2. Also noticed a slight difference in Python2 and Python3. When trying to read a locked pidfile - linux version successfully gets pid value, buton on windows python2 gets and empty string, while python3 raises an Access denied exception. Only matters for one test case.

3. Test coverage seems to be on the same 95% after including most of the python versions on windows platform, so i could remove all windows-related branch excludes.

4. And now regarding psutil. I can try copying their implementation, although it is mostly c++ code with wrappers (and i haven't really done that before) and it will complicate this module by quite a lot. Also I'm not sure, how should we properly mention psutil and their BSD license in that case?

[trbs]

I don't think it's a big deal not supporting Python 3.4 on Windows.

[trbs]

I really appreciate all your work on this ! It's a large patch so it might take me some time to have the time to dive into this and merge it.

I probably also want to migrate to pytest before merging this. Such that we can use things like this for the tests:

@pytest.mark.skipif(sys.platform == 'win32',
                    reason="does not run on windows")

[spumer]

Any updates?

[trbs]

I'm still hesitant to merge this as is. Let's try and break this up into multiple smaller PR's to make it more manageable.

I think we should be able to make the psutil library optional, using it if available and requiring it for windows support.

(If a company is reading this and wants to sponsor some of the work that would be much appreciated)

[Xorboo]

@trbs, Another long evening and done!

• psutils is now only used on windows
• removed xenial requirement (it seems to be working fine without it now)
• added CI for 3.8
• some additional changes to tests, like using tempfile instead of hardcoded /tmp dir
• Changed to use double quotation marks everywhere for consistency

One thing I haven't thought about is what should we do with DEFAULT_PID_DIR = "/var/run/" in case of Windows, I'm not really sure what folder if suitable here, any ideas? Maybe it could simply be temp folder as well? (UPD: decided to set it to temp folder, that seems reasonable to me)

PS I think this would be a nice time to bump the version I suppose? :)

[trbs]

Please check with the latest master version.

Since these where quite large changes, I hope everybody can start testing this on all platforms.

Thanks for all the help on having PidFile gain win32 support ! 🥇

[Xorboo]

Ah, pity that the PR has been split instead of merging a PR, so I don't get to have a personal commit in the repo. But I see you've made a lot of adjustments and improvements, so it doesn't matter as much, as long as we have Windows support now 😊 Thanks for taking the time, @trbs

PS I think issue #14 can potentially be closed now?

[trbs]

I'm more then happy to give you recognition ! What about creating a PR and update the AUTHORS list ?

[Xorboo]

Thanks, glad to do it, check #30

PS Also can confirm that pidfile seems to be working just fine on windows in 3.0