Handling client timeout/disconnect in a redis module

[thesecr...@developingtechnician.com]

Hello,

I'm writing a redis module, and it seems to be working well. It blocks the client for 20+ seconds (which is by design) then returns its result. However, it seems that if the client disconnects before the execution has completed, the timeout handler specified when I blocked the client is not called and instead the server crashes attempting to access memory at 0xffffffffffffffff. This error always stems from a call to RedisModule_HashGet, and all the inputs which I pass seem to be correct as far as I can tell. Is there any way to handle client disconnects safely from a redis module?

If you'd like to attempt to replicate my issue locally, you can find a minimal example (only slightly adapted from the helloblock.c example given in the api documentation) in the following gist (instructions to compile, run, and reproduce are in a comment at the top): https://gist.github.com/thesecretmaster/7b3304b9a17112ba809ca788dd51b592

Thank you for your time,
tsm

[Itamar Haber]

Hello tsm,

The crash is probably due to accessing an invalid ctx and/or freed privdata.

Look into setting a disconnect callback with `RedisModule_SetDisconnectCallback` (https://redis.io/topics/modules-api-ref#coderedismodulesetdisconnectcallbackcode) and/or `RedisModule_BlockedClientDisconnected` (https://redis.io/topics/modules-api-ref#coderedismoduleblockedclientdisconnectedcode) to manage your thread's access and behavior.

Cheers,

--
You received this message because you are subscribed to the Google Groups "Redis DB" group.
To unsubscribe from this group and stop receiving emails from it, send an email to redis-db+u...@googlegroups.com.
To post to this group, send email to redi...@googlegroups.com.
Visit this group at https://groups.google.com/group/redis-db.
For more options, visit https://groups.google.com/d/optout.

--

Itamar Haber Technicalist Evangely Phone: +972.54.567.9692

[thesecr...@developingtechnician.com]

Hi Itamar,

Thank you for your reply! I'll try using those callbacks you mentioned and then reply to this thread again with how it goes. Would disconnecting cause the ctx to become invalid? Also, I saw no mention of those callbacks in the documentation at redis-module-redoc.readthedocs.io, which is where I found the documentation I was using. Is that documentation out of date? If it is, would it be possible to update it or edit in a link so that other people are aware that it is out of date?

Thanks,

tsm

[Salvatore Sanfilippo]

Hello tms,

the problem is, to start, here:

RedisModule_HashGet(key, REDISMODULE_HASH_CFIELDS, "foosubkey", &val);

You need an end NULL parameter to terminate the list of key/pointer pairs.
Try again after fixing that to check if it works better or if the
error moved in another location (maybe there are multiple bugs).

Cheers,
Salvatore

> --
> You received this message because you are subscribed to the Google Groups "Redis DB" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to redis-db+u...@googlegroups.com.
> To post to this group, send email to redi...@googlegroups.com.
> Visit this group at https://groups.google.com/group/redis-db.
> For more options, visit https://groups.google.com/d/optout.



--

Salvatore 'antirez' Sanfilippo
open source developer - Redis Labs https://redislabs.com

"If a system is to have conceptual integrity, someone must control the
concepts."
— Fred Brooks, "The Mythical Man-Month", 1975.

[thesecr...@developingtechnician.com]

Hi Salvatore,

I just added that NULL to the end, and it didn't change anything about the error, however I think that the callbacks Itamar mentioned will be the solution to my problem. I also just realized that the docs I've been looking at (and found through google) are from some non-redis affiliated repo which hasn't been updated in over a year. I'm going to open an issue there pointing this issue out to them. Again, thank you both for your help!

tsm

[Itamar Haber]

(confession: have not reviewed the code  :)).

https://github.com/develephant/redis-module-redoc looks unmaintained - the official docs are kept at https://github.com/antirez/redis-doc and online at redis.io.

[Salvatore Sanfilippo]

Probably there are multiple bugs, in my case it immediately crashed
because of this issue when doing the tests.
Moreover there is another problem in the API usage other than the
threading issue that I yet did not check, that is, no test is made to
check that "key" is NULL in case the key you open does not exist.

[Salvatore Sanfilippo]

P.S. here there is another problem in the API usage:

void **targ = RedisModule_Alloc(sizeof(void*)*2);
targ[0] = bc;
targ[1] = (void*)(unsigned long) delay;
targ[2] = RedisModule_GetThreadSafeContext(bc);

Basically you are allocating targ for 2 items but you are storing 3.
I did the API changes I reported and tested the module and I can't see
it crashing. Anyway what the module does here is very odd because it
uses a thread and a context but then it locks the thread safe context
for all the time the loop runs, with the sleep inside, so Redis main
thread remains blocked.

[thesecr...@developingtechnician.com]

Thank you both again! I've got my code working now. There was a RedisModuleString that was getting mysteriously freed in the middle of execution. Would you accept a PR to the redis-doc repo to make it compatible with readthedocs.io, so there's an up to date readthedocs page for redis?

tsm