[hunchentoot-devel] thread safety in session-value
Hello. Just noticed in the sources that SESSION-VALUE code does not have WITH-LOCK. IMHO it is necessary here. Best regards, -Anton
on Wednesday, September 3, 2008, 11:51:25 PM Hans wrote:
On Wed, Sep 3, 2008 at 22:32, Anton Vodonosov <avodonosov@yandex.ru> wrote:
Just noticed in the sources that SESSION-VALUE code does not have WITH-LOCK. IMHO it is necessary here.
Can you please be more specific? Thanks.
I thought that several functions work with the same list - SESSION-DATA slot of the session. For example if DELETE-SESSION-VALUE will happen in parallel with with (SESSION-VALUE ...) we will have ASSOC and DELETE on the same list simultaneously, which is IMHO unsafe. (SETF (SESSION-VALUE)..)) with (DELETE-SESSION-VALUE looks unsafe too: SETF does (push <something> (slot-value ,%session 'session-data)) and in the same time in the DELETE-SESSION-VALUES (setf (slot-value session 'session-data). PUSH is not necessary atomic, IMHO; is it safe to use it in parallel with SETF? P.S. DELETE-SESSION-VALUE should use look too. - Anton
On Wed, Sep 3, 2008 at 23:22, Anton Vodonosov <avodonosov@yandex.ru> wrote:
on Wednesday, September 3, 2008, 11:51:25 PM Hans wrote:
On Wed, Sep 3, 2008 at 22:32, Anton Vodonosov <avodonosov@yandex.ru> wrote:
Just noticed in the sources that SESSION-VALUE code does not have WITH-LOCK. IMHO it is necessary here.
Can you please be more specific? Thanks.
I thought that several functions work with the same list - SESSION-DATA slot of the session.
I agree, this could be a problem if one session is accessed by multiple requests, and this is not a completely unlikely scenario (although I have not been bitten by this). Do you have a patch? -Hans
on Thursday, September 4, 2008, 7:47:38 AM Hans wrote:
On Wed, Sep 3, 2008 at 23:22, Anton Vodonosov <avodonosov@yandex.ru> wrote:
on Wednesday, September 3, 2008, 11:51:25 PM Hans wrote:
On Wed, Sep 3, 2008 at 22:32, Anton Vodonosov <avodonosov@yandex.ru> wrote:
Just noticed in the sources that SESSION-VALUE code does not have WITH-LOCK. IMHO it is necessary here.
Can you please be more specific? Thanks.
I thought that several functions work with the same list - SESSION-DATA slot of the session.
I agree, this could be a problem if one session is accessed by multiple requests, and this is not a completely unlikely scenario (although I have not been bitten by this). Do you have a patch?
No, sorry, I don't have a patch at the moment (if I had had it I would have posted it in the previous letter). Best regards, -Anton
On Thu, 4 Sep 2008 06:47:38 +0200, "Hans Hübner" <hans@huebner.org> wrote:
I agree, this could be a problem if one session is accessed by multiple requests, and this is not a completely unlikely scenario (although I have not been bitten by this). Do you have a patch?
I don't think this needs a patch. There's a lock for the case that Hunchentoot accesses the global session database. If you (as a user) write an application where different handlers or different threads work on the same session object at the same time, then you know what you're doing and you should take care of locking (if needed) yourself. IMNSHO it is a bad decision to defensively wrap locks around everything that could in theory be accessed simultaneously.
on Thursday, September 4, 2008, 10:37:39 PM Edi wrote:
On Thu, 4 Sep 2008 06:47:38 +0200, "Hans Hübner" <hans@huebner.org> wrote:
I agree, this could be a problem if one session is accessed by multiple requests, and this is not a completely unlikely scenario (although I have not been bitten by this). Do you have a patch?
I don't think this needs a patch. There's a lock for the case that Hunchentoot accesses the global session database. If you (as a user) write an application where different handlers or different threads work on the same session object at the same time, then you know what you're doing and you should take care of locking (if needed) yourself. IMNSHO it is a bad decision to defensively wrap locks around everything that could in theory be accessed simultaneously.
Yes, such a contract may be used too, although in this case it looks to me that user must take care of locking in virtually 100% of cases - even single handler may be called from different threads for the same session. And btw this is unexpected clarification to session functions behavior, at least for me. In fact my attention to this question was drawn by the fact that (SETF (SESSION-VALUE )) uses lock, but other functions do not. And the condition that aimed to be protected by the lock in (SETF (SESSION-VALUE )) may potentially be broken by concurrent DELETE-SESSION-VALUE. Best regards, -Anton P.S. If you wonder why am I poking in the hunchentoot code - I am studying how session keys are generated in hope to find an example of secure random value generation to use it in cl-openid.
On Thu, 4 Sep 2008 23:36:01 +0300, Anton Vodonosov <avodonosov@yandex.ru> wrote:
Yes, such a contract may be used too, although in this case it looks to me that user must take care of locking in virtually 100% of cases - even single handler may be called from different threads for the same session.
Yes, you're right. I never had problems with this so far, but maybe this is because I've rarely used Hunchentoot with SMP Lisps. In this case, you don't want to use a global lock, though, and I wonder how expensive it is to have one lock per session. I have to think about this a bit...
In fact my attention to this question was drawn by the fact that (SETF (SESSION-VALUE )) uses lock, but other functions do not.
The lock in (SETF SESSION-VALUE) is clearly unneeded as START-SESSION already has a lock. As the name says, *SESSION-DATA-LOCK* was only meant to protect *SESSION-DATA*.
If you wonder why am I poking in the hunchentoot code
Oh, that's fine. Feel free to poke around and find more bugs... :)
on Friday, September 5, 2008, 12:14:59 AM Edi wrote:
In this case, you don't want to use a global lock, though, and I wonder how expensive it is to have one lock per session. I have to think about this a bit...
One per session is better IMO; lock in the worst case is several bytes + maybe a string if you will provide a name - quite cheap. Best regards, -Anton
participants (3)
-
Anton Vodonosov -
Edi Weitz -
Hans Hübner