Describe the bug

Detected with valgrind - after calling MODULE LOAD command for loading a module where loading fails due to REDISMODULE_ERR status, we expect that server events to which the module subscribed prior to the failure upon loading will not be handeled. However, this is not the behavior, and handling these events causes accessing invalid memory.

To reproduce

  1. Launch redis-server without modules.
  2. Call module load command and the RedisModule_OnLoad callback in the module code fails (returning REDISMODULE_ERR). As part of the cleanup, module context is freed (moduleFreeModuleStructure(ctx.module);)
  3. Before the failure, upon initialization, module was registered to RedisModuleEvent_FlushDB event.
  4. Call FLUSHALL command. emptyData is called -> moduleFireServerEvent is called and the callback of the module that was sent to the RedisModuleEvent_FlushDB is called as it is still registered.
  5. moduleFireServerEvent accessing the module context that was freed.

Expected behavior

Modules' callbacks will not be called if nodule loading has failed.

Comment From: oranagra

thanks for reporting. i see that if the loading fails on a later stage of moduleLoad, we do call moduleUnload which handles this un-registration. but when it fails on the RedisModule_OnLoad call, we only un-register several specific things and this one is missing.

i suppose we should unify them by calling moduleLoad, passing it some hint to tell it not to call the RedisModule_OnUnload symbol in that case.

Comment From: enjoy-binbin

i suppose we should unify them by calling moduleLoad

@oranagra I tried unified it with moduleUnload, but it was not easy to change or there would be a lot of diff, so i submitted this version first #12809.