File-based session does not store any data on Windows

[mizutori]

Django 1.0 (or trunk r9029) has a bug that does not store any session data using file-based sessions on Windows system.


[1] Sample code

To use file-based sessions, set the SESSION_ENGINE setting to "django.contrib.sessions.backends.file". On Windows system, the following code does not work properly. The session data for "sess_key" will never be passed to the next index2 session.

---[ views.py ]---
def index(request):
    request.session['sess_key'] = "yes"
    request.session.modified = True
    return HttpResponseRedirect('index2.html')

def index2(request):
    print "session key="+repr(request.session['sess_key'])
    return HttpResponseRedirect('index3.html')
---

[2] My analysis

This is a bug of save() function of django.contrib.sessions.backends.file.SessionStore class. On Windows, os.rename function of Python will raise OSError if the destination file already exists. Whereas on Unix, it will be removed silently. To fix this problem, remove the existing file before renaming if the current operating system is Windows.


[3] My workaround

Apply unlink() function before rename() if the operating system is Windows to fix the save() function of file.SessionStore class.

---[ django/contrib/sessions/backends/file.py ]---
    def save(self, must_create=False):
        ...
        try:
            ...
#00122:
+           if os.name == "nt":
+               os.unlink(session_file_name)
            os.rename(output_file_name, session_file_name)
            renamed = True
---
# added patch code with "+" mark

[Malcolm Tredinnick]

Whoops. Yes, you're right. os.rename() does need a clear landing zone on Windows. There's been talk from time to time of creating atomic_rename or something similar in Python that is cross-platform, but it doesn't exist yet.

[Malcolm Tredinnick]

This is possibly going to be a bit tricky to solve. There are two race conditions that we need to worry about: firstly if another process comes in and reads the session file, the idea is that they see old data, not totally missing data. On windows, there's a small window in which another process will read between the unlink and the rename, which will be a bit confusing to the user.

Secondly, you can't unlink a file that's open in Windows, so there's the potential that the unlink will raise an error (again, because another process is coming in with the same session ID, perhaps due to multiple clicks of a browser button).

In both cases, it might be a case of tough luck -- we catch the error in the latter case and no save happens. But it'd be nice to avoid this if possible. Not sure how, though. Reintroducing locking there isn't going to be at all pleasant, since it's unbelievably tricky to get right in a portable fashion.

[(none)]

Milestone post-1.0 deleted

[Malcolm Tredinnick]

I'm not going to have time to work on this for a bit and I want to clean out my list of "reserved" bugs. So giving it back to the pool. Thoughts in comment:2 are still valid, I feel. It's tricky, but I'm happy to review any patches that come along.

[anonymous]

This still doesn't work in django 1.2

[Julien Phalip]

Patch needs improvement as per Malcolm's comments.

[Jannis Leidel]

Please provide a proper patch. This also needs tests.

[anonymous]

This still happens in 1.4.1. Unfortunately it makes file-based sessions useless on Django in Windows (i.e., development or whatever). Also, it's hard to find, as the exception is caught without warning -- this wouldn't be so bad if it broke noisily.

[Aymeric Augustin]

​os.rename hasn't improved on Windows. Would it make sense to use ​shutil.copyfile instead when the platform is Windows?

This isn't atomic, and if I understand correctly, it may still be vulnerable to exceptions if the destination is open for reading at the wrong time.

But we can't do anything about that, besides telling people to use a better OS — ie. POSIX-compliant.

[Joeri Bekker]

I made a pull request ​https://github.com/django/django/pull/823 that changes os.rename to shutil.move which does (unlike the Python documentation indicates) an atomic os.rename but falls back to "the best Windows got to an atomic rename" in this specific case: shutil.copy2 and os.unlink.

The file based session save function is actually tested and fails on Windows:

(env) D:\Work\Projects\joeri\django\tests>python runtests.py --settings=test_sqlite sessions
Creating test database for alias 'default'...
Creating test database for alias 'other'...
........................................................................................x........................................................................................F................................
======================================================================
FAIL: test_clearsessions_command (django.contrib.sessions.tests.FileSessionTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Work\Projects\joeri\django\django\test\utils.py", line 227, in inner
    return test_func(*args, **kwargs)
  File "D:\Work\Projects\joeri\django\django\contrib\sessions\tests.py", line 444, in test_clearsessions_command
    self.assertEqual(1, count_sessions())
AssertionError: 1 != 2

----------------------------------------------------------------------
Ran 210 tests in 0.276s

FAILED (failures=1, expected failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

After patch:

(env) D:\Work\Projects\joeri\django\tests>python runtests.py --settings=test_sqlite sessions
Creating test database for alias 'default'...
Creating test database for alias 'other'...
........................................................................................x.........................................................................................................................
----------------------------------------------------------------------
Ran 210 tests in 0.235s

OK (expected failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

[Aymeric Augustin <aymeric.augustin@…>]

In b9cc61021a0db1e5b41e61d3e53180e4fc618f9c:

Fixed #9084 - Best approach for an OS to atomically rename the session file.

[spyjamesbond0072003@…]

Django 1.5.1 still not fix

[Aymeric Augustin]

It's fixed in master.

[anonymous]

How to solve it? Django 1.5.1 still not fix

[anonymous]

Confirmed, still not working in 1.5.1.

[Simon Charette]

The problem is fixed in ​master and 1.6a1.

It won't be backported to 1.5.X since ​it's not considered critical.

[me21@…]

A new function is introduced in Python 3.3, os.replace(), which renames files atomically and cross-platformly: ​http://bugs.python.org/issue8828
It might be possible to use this function in future, when Django support for Python 2.x is abandoned, or when os.replace() is (ever) backported to Python 2.x.

[me21@…]

In fact, since MoveFileEx API is considered to be atomic (for example, Java uses it: ​http://bugs.python.org/issue8828#msg146307), Django could use it through ctypes interface even now (​https://github.com/mitsuhiko/python-atomicfile/ - _rename() function - and disregard that its author believes it to be non-atomic, it seems to be atomic actually). The os.replace() function in Python 3.3 is implemented through MoveFileEx anyway.