Bug #97497 for Catalyst-Plugin-Session: regression in Catalyst::Plugin::Session (since ver. 0.35)

Fri Jul 25 07:50:59 2014 gregoa [...] cpan.org - Ticket created

From:	gregoa [...] cpan.org
Subject:	regression in Catalyst::Plugin::Session (since ver. 0.35)

We have the following bug reported to the Debian package of Catalyst-Plugin-Session (https://bugs.debian.org/756000): It doesn't seem to be a bug in the packaging, so you may want to take a look. Thanks! ------8<-----------8<-----------8<-----------8<-----------8<----- Package: libcatalyst-plugin-session-perl Severity: minor Tags: patch Hi, In the version 0.35 of Session.pm, the method "calculate_initial_session_expires" has been changed and now it tries to get a stored session_expired if sessionid exists. One of its usage is inside reset_session_expires() which is called during a session creation. There is no real need to ask for session data here because the session was just created. In most cases, an additional get_session_data() call costs nothing, but we are using Infinispan as session store for caching and such call means at least one or even several remote requests returning no data. The simple fix for this problem can be as follows. sub create_session_id { my $c = shift; my $sid = $c->generate_session_id; $c->log->debug(qq/Created session "$sid"/) if $c->debug; - $c->_sessionid($sid); $c->reset_session_expires; + $c->_sessionid($sid); $c->set_session_id($sid); return $sid; } -- System Information: Debian Release: jessie/sid APT prefers unstable APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 3.14-1-amd64 (SMP w/8 CPU cores) Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash -- Martin Zobel-Helas <zobel@debian.org> Debian System Administrator Debian & GNU/Linux Developer Debian Listmaster http://about.me/zobel Debian Webmaster GPG Fingerprint: 6B18 5642 8E41 EC89 3D5D BDBB 53B1 AC6D B11B 627B ------8<-----------8<-----------8<-----------8<-----------8<----- Thanks for considering, gregor herrmann, Debian Perl Group

Fri Jul 25 07:51:56 2014 gregoa [...] cpan.org - Broken in 0.35 added

Mon Jun 22 13:36:45 2015 jjnapiork [...] cpan.org - Correspondence added

On Fri Jul 25 07:50:59 2014, GREGOA wrote: Show quoted text

> We have the following bug reported to the Debian package of > Catalyst-Plugin-Session (https://bugs.debian.org/756000): > > It doesn't seem to be a bug in the packaging, so you may want to take > a look. Thanks! > > ------8<-----------8<-----------8<-----------8<-----------8<----- > > Package: libcatalyst-plugin-session-perl > Severity: minor > Tags: patch > > Hi, > > In the version 0.35 of Session.pm, the method > "calculate_initial_session_expires" has been changed and now it tries > to > get a stored session_expired if sessionid exists. One of its usage is > inside reset_session_expires() which is called during a session > creation. > > There is no real need to ask for session data here because the > session was just created. > > In most cases, an additional get_session_data() call costs nothing, > but > we are using Infinispan as session store for caching and such call > means > at least one or even several remote requests returning no data. The > simple fix for this problem can be as follows. > > sub create_session_id { > my $c = shift; > > my $sid = $c->generate_session_id; > > $c->log->debug(qq/Created session "$sid"/) if $c->debug; > > - $c->_sessionid($sid); > $c->reset_session_expires; > + $c->_sessionid($sid); > $c->set_session_id($sid); > > return $sid; > } > > > > > -- System Information: > Debian Release: jessie/sid > APT prefers unstable > APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable'), > (1, 'experimental') > Architecture: amd64 (x86_64) > Foreign Architectures: i386 > > Kernel: Linux 3.14-1-amd64 (SMP w/8 CPU cores) > Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) > Shell: /bin/sh linked to /bin/dash

Hi, jnap here, I'd like to help but I am having trouble understanding. I don't think "calculate_initial_session_expires" was changed between .34 and .35 but rather it was added. I'm not totally sure why, I'll see if the author of that change remembers. I'm a little nervous to just add that extra line to the code since I can't figure all the side effects, the session code has had a lot of hands and is widely deployed (and tons of people have local subclasses, etc). Can you help me think of a smaller change that solves this for you? Also I'm not totally sure I get the issue. Its not broken just noisy at creation (when session is first created it polls the session storage too much?) Does that only happen at init time, or all the time? jnap

Mon Jun 22 13:36:48 2015 The RT System itself - Status changed from 'new' to 'open'

Mon Jun 22 13:44:22 2015 gregoa [...] debian.org - Correspondence added

CC:	Martin Zobel-Helas <zobel [...] debian.org>
Subject:	Re: [rt.cpan.org #97497] regression in Catalyst::Plugin::Session (since ver. 0.35)
Date:	Mon, 22 Jun 2015 19:43:59 +0200
To:	John Napiorkowski via RT <bug-Catalyst-Plugin-Session [...] rt.cpan.org>
From:	gregor herrmann <gregoa [...] debian.org>

On Mon, 22 Jun 2015 13:36:50 -0400, John Napiorkowski via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=97497 >

Show quoted text

> On Fri Jul 25 07:50:59 2014, GREGOA wrote:

> > We have the following bug reported to the Debian package of > > Catalyst-Plugin-Session (https://bugs.debian.org/756000): > > > > It doesn't seem to be a bug in the packaging, so you may want to take > > a look. Thanks! > > > > ------8<-----------8<-----------8<-----------8<-----------8<----- > > > > Package: libcatalyst-plugin-session-perl > > Severity: minor > > Tags: patch > > > > Hi, > > > > In the version 0.35 of Session.pm, the method > > "calculate_initial_session_expires" has been changed and now it tries > > to > > get a stored session_expired if sessionid exists. One of its usage is > > inside reset_session_expires() which is called during a session > > creation. > > > > There is no real need to ask for session data here because the > > session was just created. > > > > In most cases, an additional get_session_data() call costs nothing, > > but > > we are using Infinispan as session store for caching and such call > > means > > at least one or even several remote requests returning no data. The > > simple fix for this problem can be as follows. > > > > sub create_session_id { > > my $c = shift; > > > > my $sid = $c->generate_session_id; > > > > $c->log->debug(qq/Created session "$sid"/) if $c->debug; > > > > - $c->_sessionid($sid); > > $c->reset_session_expires; > > + $c->_sessionid($sid); > > $c->set_session_id($sid); > > > > return $sid; > > }

> > jnap here, I'd like to help but I am having trouble understanding. > I don't think "calculate_initial_session_expires" was changed > between .34 and .35 but rather it was added. I'm not totally sure > why, I'll see if the author of that change remembers. I'm a little > nervous to just add that extra line to the code since I can't > figure all the side effects, the session code has had a lot of > hands and is widely deployed (and tons of people have local > subclasses, etc). Can you help me think of a smaller change that > solves this for you? > > Also I'm not totally sure I get the issue. Its not broken just > noisy at creation (when session is first created it polls the > session storage too much?) Does that only happen at init time, or > all the time?

jnap, thanks for picking up this issue! I'm cc'ing zobel, the original submitter, hoping he can help out with the details (I'm just the messanger :)) Cheers, gregor -- .''`. Homepage: http://info.comodo.priv.at/ - OpenPGP key 0xBB3A68018649AA06 : :' : Debian GNU/Linux user, admin, and developer - https://www.debian.org/ `. `' Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe `- NP: REM: Radio Song

Download signature.asc
application/pgp-signature 949b

Message body not shown because it is not plain text.