kemal-session icon indicating copy to clipboard operation
kemal-session copied to clipboard

File session can die if file removed

Open rdp opened this issue 7 years ago • 5 comments

set the

   config.timeout = Time::Span.new(days: 0, hours: 0, minutes: 10, seconds: 0)

then access your web server so the cookie file is created.

Now wait one minute so the File.utime stuff will be triggered (or possibly until a gc fires that cleans it up?)

Delete the file.

Access your web server.

Error setting time to file './sessions/58c9d216c8ba7334f0bd875798feb61f.json': No such file or directory (Errno) 0x102681975: *CallStack::unwind:Array(Pointer(Void)) at ?? 0x102681911: *CallStack#initialize:Array(Pointer(Void)) at ?? 0x1026818e8: *CallStack::new:CallStack at ?? 0x102656181: *raise<Errno>:NoReturn at ?? 0x1026df2f3: *File::utime<Time, Time, String>:Nil at ?? 0x1027b06de: *Session::FileEngine#is_in_cache?<String>:Bool at ?? 0x1027b07f9: *Session::FileEngine#string?<String, String>:(String | Nil) at ?? 0x1027a0eed: *Session#string?<String>:(String | Nil) at ?? 0x1026783fa: ~procProc(HTTP::Server::Context, String)@kemal_server.cr:456 at ??

though it does recover a bit after that.

I'm also not sure how I feel about the file stuff being cached at all, since the cache will persist across requests which means if it's in front of a load balancer, with a shared file system, it wouldn't persist across servers. Maybe this should be noted somewhere? Or I think ideally it would at least re-read "in" the file at the beginning of each request...or at least compare mtime with the last known mtime. Or else be made clear that it doesn't work today with shared storage (or perhaps I'm wrong?)

Cheers!

rdp avatar Apr 13 '17 22:04 rdp

In retrospect, even if you're running two servers on the same box, and using the file backing, the two can get out of sync if they both cache the same session from the file system...so much difficulty makes me think to propose that it never cache but just always re-read from the file system. It also seems that destroy_all doesn't work because of the cache (leaves cached one undestroyed). Though unrelated...FWIW...

rdp avatar Apr 30 '17 00:04 rdp

In addition, it's not hard to imagine a race condition scenario today, when servicing multi simultaneous requests, these lines:

          load_into_cache(session_id) unless is_in_cache?(session_id)
          return @cache.{{name.id}}(k)

could be run interleaved by different fibers, and return the value from a different session [?]

rdp avatar May 02 '17 17:05 rdp

File storage also has an issue when utime fails. I have this working on a development server inside a vm. The storage folder is on the host, and doesn't seem to support changing the time.

So it throws an exception every time is_in_cache is called which I think is once a minute.

On the live server, without the vm / host drive, it works fine.

As for the issues above, a note to say file sessions are not suitable for apps with multiple instances would probably be enough. If you're app needs multiple instances, it probably also needs redis based sessions?

crisward avatar May 04 '17 11:05 crisward

I agree with @crisward that something like redis or memcached will be more suitable for apps with multiple instances

sdogruyol avatar May 04 '17 11:05 sdogruyol

Yeah or mysql is a typical one. I think the original problem is still present even if run on only one instance however...

On Thu, May 4, 2017 at 5:57 AM, Serdar Dogruyol [email protected] wrote:

I agree with @crisward https://github.com/crisward that something like redis or memcached will be more suitable for apps with multiple instances

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kemalcr/kemal-session/issues/45#issuecomment-299164808, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAw0OjT8A8OYTsAo1IWKWJk_FRB5LhTks5r2b0_gaJpZM4M9Suc .

rdp avatar May 22 '17 15:05 rdp