Clarification on Redis session storage and locking

[vavra5]

Hello,

lua-resty-session= 3.7 lua-resty-openidc = 1.7.3.1

Can you provide some clarification on the session options for Redis storage adapter? I'm unclear on the usage of 'session_redis_spinlockwait' and 'session_redis_maxlockwait'. Through my testing it seems that 'session_redis_maxlockwait' is the time that the session will be locked from concurrent write attempts.

Due to other factors we need to consider switching from Client Side Cookie session to a Server Side Session storage. Client Side Cookie has worked well for us up to this point.

My scenario involves a website that when a page request is made multiple XHR calls can be in flight at nearly the same time. The behavior I've noticed is that the page loads slow because the first request locks the session (expected behavior) and the other request waits for it to be unlocked. I see this when using default or regenerate strategies.

Now, if I turn session locking off the page loads very fast. The site behaves the same as Client Side Cookie session, but there is one thing that doesn't seem right. I'm using the 'lua-resty-openidc' for authentication. When I login and open 2+ tabs in the browser. All is well, the session is the same in all tabs. When I allow the access_token to expire and refresh all the tabs at the same time, I eventually lose my authentication when all tabs are done reloading. I've tried this with both default and regenerate strategies. I've read previous issues in this repository but I'm still unsure on what may be happening here.

I can provide more details if the above isn't clear.

Thanks.

[fastcoding]

I got the same locking issue - if lock is enabled on redis session, reloading the page will hang for session_redis_maxlockwait seconds.

I guess there should be some bug with encryption/decryption. If i set lock to off and session_cipher to none, most of time I still get "invalid signature" error that occurs in strategies/default.lua:37:

if session.hmac(hkey, input) ~= hash then
 return nil, "cookie has invalid signature"
end

switching to cookie based session, all errors are gone, but I need redis to cache my session.

[bungle]

And you are both using lua-resty-openidc? Can you be sure that lua-resty-openidc in all circumstances when it calls start, it does call close or save? Even in error scenarios? If that is not the case, the library will leave session locked.

[bungle]

If I read the code right, the only places where it starts the session are: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1393 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1516

And only place where it releases the lock are: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L378 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1184

Are at least one of the last two lines always executed when one of the first two lines are executed? It is not very obvious that it is always the case (let's not focus on runtime errors, because that is surely a case that the code does not handle, but that should probably been reported as a bug to the project).

The last save line has before that :save: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1082 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1090 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1095 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1103 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1111 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1130 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1135

So let's next look how the openidc_authorization_response is called: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1414

That looks like the session started at: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1393

Is never closed or saved, and thus lock is not released if any of these happen: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1082 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1090 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1095 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1103 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1111 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1130 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1135

So on error case I suggest the lua-resty-openidc calls session:close.

[bungle]

Also this line: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1516

Opens a session and locks it.

Then goes to: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1298

And so there are also: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1301 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1305 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1308 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1311 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1319 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1332 https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1339

Before it finally closes the thing there: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1363

So all those places can lead to session staying locked.

[vavra5]

@bungle Thanks for your thorough response and indentifying this issue in lua-resty-openidc. I will watch the issue over in the lua-resty-openidc repository for the resolution.

I read through the code a little more in redis.lua. I think now I have a clearer picture of 'session_redis_spinlockwait' and 'session_redis_maxlockwait'.

[bungle]

@vavra5, yes, it waits more and more on each iteration. If you see wait as long as max wait, you surely have start not closed, or extremely slow performance (which is less likely).

[bungle]

Closing this now as I think we got it figured out. Please reopen if needed or more questions arise.