@warriorguo It looks like missing a test for module unload while the timer is still running.

@sundb thanks for the review, actually I tried to add a test for unloading as it would return an error, but it would be caught by (https://github.com/redis/redis/blob/unstable/tests/test_helper.tcl#L850) and caused the failure. Any suggestions for this situation?

@warriorguo You can post or commit your test code, I will help to see it.

@sundb please check 2f8fd8a. the failure message was something like.

[exception]: Executing test client: ERR Error unloading module: operation not possible..
ERR Error unloading module: operation not possible.
    while executing
"[srv $level "client"] {*}$args"
    (procedure "r" line 7)
    invoked from within
"r module unload timer"
    ("uplevel" body line 8)
    invoked from within
"uplevel 1 $code"
    (procedure "test" line 51)
    invoked from within
"test "Module can be unloaded only when timer was finished" {
        r set "timer-incr-key" 0
        set id [r test.createtimer 2000 timer-incr-key]
..."
    ("uplevel" body line 57)
    invoked from within
"uplevel 1 $code "
    (procedure "start_server" line 3)
    invoked from within
"start_server {tags {"modules"}} {
    r module load $testmodule

    test {RM_CreateTimer: a sequence of timers work} {
        # We can't guarantee s..."
    (file "tests/unit/moduleapi/timer.tcl" line 3)
    invoked from within
"source $path"
    (procedure "execute_test_file" line 4)
    invoked from within
"execute_test_file $data"
    (procedure "test_client_main" line 10)
    invoked from within
"test_client_main $::test_server_port "

thanks in advance!

@warriorguo @sundb isn't it better to delete the timer when the module is unloaded (rather than refuse the module unload)?

@oranagra I thought about that, but I saw that returned an error when blocked_clients was not empty, and I gave up on that idea.

@oranagra in the test case, deleting the timer directly would cause the memory leaks as well, to the ones were held by the parameter.

@oranagra in the test case, deleting the timer directly would cause the memory leaks as well, to the ones were held by the parameter.

The keyname here is leaked? User data provided to the timer needs to be freed...

    RedisModuleString *keyname;
    if (RedisModule_StopTimer(ctx, id, (void **) &keyname) == REDISMODULE_OK) {
        RedisModule_FreeString(ctx, keyname);

@zuiderkwast yes, in the case, if the timer was deleted automatically when the module unload, the user data would be leaked that would have been supposed to be handled at (https://github.com/redis/redis/blob/unstable/tests/modules/timer.c#L12)

triggered daily CI cycle for module tests:
https://github.com/redis/redis/actions/runs/1778235529