Django

Code

Ticket #9084 (new)

Opened 2 years ago

Last modified 5 months ago

File-based session does not store any data on Windows

Reported by: mizutori Assigned to:
Milestone: Component: django.contrib.sessions
Version: 1.1 Keywords:
Cc: wiser0+djangoproject@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

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

Attachments

Change History

09/16/08 22:00:57 changed by mtredinnick

  • status changed from new to assigned.
  • needs_better_patch changed.
  • needs_tests changed.
  • owner changed from nobody to mtredinnick.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.

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.

09/17/08 04:49:15 changed by mtredinnick

  • summary changed from [BUG] file-based session does not store any data on Windows to File-based session does not store any data on Windows.

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.

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

02/27/09 22:53:55 changed by mtredinnick

  • owner deleted.
  • status changed from assigned to new.

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.

10/21/09 11:53:49 changed by wiser0

  • cc set to wiser0+djangoproject@gmail.com.
  • version changed from 1.0 to 1.1.

10/21/09 11:59:13 changed by wiser0

  • has_patch set to 1.

Add/Change #9084 (File-based session does not store any data on Windows)




Change Properties
Action