Threading bug, changes to pipelining, defcmd, expect, ability to use connection pooling.

[JonathanSmith]

Hi there,

I was trying make cl-redis work in a multithreaded environment with connection pooling, and I was having a bit of difficulty, so I fixed a few things in this patch.

Basically, the issues were: 1.) using (Setf (fdefinition ...) (lambda () ...) isn't thead safe, so I rearranged the code to avoid that. 2.) The socket input streams weren't getting flushed after expect, so in certain situations reusing connections I would get 'garbage' replies. 3.) There was a bug in the expect method, where if you put a 0 length string into the database and pulled it out, the code would still try to access the 0th element in it, which was not good.

I haven't run the tests (I tried but I got a character encoding error). These fixes are working for my web application that I've been working on, so I think they should be upstreamed.

Thanks,

-Jon

[vseloved]

Thanks a lot for reporting these problems! Particularly the not thread-safe issue with fdefinition is a shame. Especially, since the implementation with special variables is much simpler and should be faster.

I've pushed the new version with all the changes, although I've implemented them somewhat differently. I don't particularly like the idea of separation of expect facade and %expect implementation, because both should be exported (to allow adding new expect methods), and that makes the API more confusing. And all that can be implemented with an :around method on expect, which I have done.

Regarding running the tests, you should have UTF-8 encoding, because there are special tests to check, that multi-byte characters are handled correctly (for SLIME:

(setq slime-net-coding-system 'utf-8-unix)

I'd be grateful, if you test this variant and report any problem you might encounter.

[JonathanSmith]

Yes, I think your expect fix is more elegant than mine. I'll update my environment to use UTF-8 and try the newest version! Thanks!

[JonathanSmith]

Looks like the most recent revision does the trick. Only comment is that SBCL complains because the defvars for pipelined pipeline are after the variable is used in the around methods.

It can be fixed by moving them to the top of the file.

; file: /home/jon/github/cl-redis/redis.lisp ; in: DEFMETHOD REDIS:EXPECT :AROUND (T) ; (PUSH TYPE REDIS::PIPELINE) ; --> LET* ; ==> ; (SETQ REDIS::PIPELINE #:NEW319) ; ; caught WARNING: ; undefined variable: REDIS::PIPELINE

; (IF REDIS::PIPELINED ; (PROGN (PUSH TYPE REDIS::PIPELINE) :PIPELINED) ; (CALL-NEXT-METHOD)) ; ; caught WARNING: ; undefined variable: REDIS::PIPELINED

; in: DEFMETHOD REDIS:TELL :AROUND ((EQL 'REDIS::SELECT)) ; (IF REDIS::PIPELINED ; (ERROR "Can't use SELECT inside WITH-PIPELINING.") ; (CALL-NEXT-METHOD)) ; ; caught WARNING: ; undefined variable: REDIS::PIPELINED ; ; compilation unit finished ; Undefined variables: ; REDIS::PIPELINE REDIS::PIPELINED ; caught 3 WARNING conditions

[vseloved]

Thanks, did that in the latest version. Also added the new Redis commands, that have appeared recently.