RM_Fork clobbers errno before returning, so a module cannot tell a busy fork slot from a real fork failure #15612

Describe the bug

RM_Fork() sets a meaningful errno and then logs before returning, and the logging can overwrite it. A module that inspects errno after RedisModule_Fork() returns -1 therefore cannot reliably distinguish "another child is already running" from a genuine fork failure.

The chain, as of 8f36533:

1. redisFork() reports the mutually-exclusive-child case through errno (src/server.c):

int redisFork(int purpose) {
    if (isMutuallyExclusiveChildType(purpose)) {
        if (hasActiveChildProcess()) {
            errno = EEXIST;
            return -1;
        }

2. RM_Fork() logs on failure and returns, with nothing preserving errno across the log call (src/module.c):

int RM_Fork(RedisModuleForkDoneHandler cb, void *user_data) {
    pid_t childpid;

    if ((childpid = redisFork(CHILD_TYPE_MODULE)) == 0) {
        ...
    } else if (childpid == -1) {
        serverLog(LL_WARNING,"Can't fork for module: %s", strerror(errno));
    }
    ...
    return childpid;
}

3. serverLogRaw() performs file I/O, which sets errno when it fails (src/server.c):

fp = log_to_stdout ? stdout : fopen(server.logfile,"a");
if (!fp) return;

So if the logfile cannot be opened or written — log directory removed, disk full, fd limit reached — errno becomes ENOENT/ENOSPC/EMFILE and EEXIST is lost. RM_Fork still returns -1, but the reason is gone.

Impact

A module that wants to wait for the slot rather than skip its work has no reliable way to recognise the condition it should wait for. RediSearch's fork GC is one such caller: it retries while the failure is EEXIST, since a BGSAVE, AOF rewrite or replication fork is transient, and gives up otherwise. Under the conditions above it misclassifies a busy slot as permanent and abandons a collection it was explicitly asked to perform.

Note the hazard is conditional on the log actually being written: serverLogRaw returns early when level < server.verbosity, so a build or configuration that suppresses LL_WARNING will not clobber anything. That makes the failure load- and configuration-dependent, which is the awkward part — a module cannot test for it.

Expected behavior

errno set by redisFork() survives until RM_Fork() returns, so errno == EEXIST is a dependable test for "another child is active".

Suggested fix

Preserving errno around the log call in RM_Fork is the narrow fix:

    } else if (childpid == -1) {
        int fork_errno = errno;
        serverLog(LL_WARNING,"Can't fork for module: %s", strerror(fork_errno));
        errno = fork_errno;
    }

It may be worth doing this inside serverLogRaw instead, saving and restoring errno around its I/O. serverLog is called from error paths throughout the codebase that go on to use errno or strerror(errno), so the same clobber is reachable well beyond this one function; fixing it centrally removes the class rather than one instance.

Happy to open a PR for either shape if you have a preference.

Additional information

An alternative would be to expose the active-child state to modules directly — something a module could check under the GIL before calling RedisModule_Fork — but that adds API surface where preserving errno would do.