Destructor for generic/top-level client in plugin not called on conditional restart

[LennartF22]

Describe the bug

When performing a conditional restart (SIGUSR1) of an OpenVPN server with a plugin, openvpn_plugin_client_constructor_v1 is called to create a new generic/top-level client instance without deconstructing the previous one with openvpn_plugin_client_destructor_v1. When the server is stopped after one or more conditional restarts, only the last instance is destructed.

This not only causes a memory leak, it also creates a logical issue when trying to distinguish the allocation of a generic/top-level client and a real client early on by counting allocations/deallocations.

To reproduce

1. Write a simple OpenVPN plugin (see below for a minimal example).
   • Make an allocation in openvpn_plugin_client_constructor_v1 and return a pointer to it.
   • Free the allocation in openvpn_plugin_client_destructor_v1.
2. Start an OpenVPN server with the plugin loaded.
3. Trigger a conditional restart of the OpenVPN server via SIGUSR1 to observe the issue.

Expected behavior

On a conditional restart, either openvpn_plugin_client_constructor_v1 should not be called, or openvpn_plugin_client_destructor_v1 should be called beforehand, so that there is no memory leak.

Version information

• OS: Debian Bookworm
• OpenVPN version: 2.6.13 (also 2.5.9)

Additional context

Minimal example:

#include <openvpn/openvpn-plugin.h>

#define PLUGIN_NAME         "Issue Demo"

OPENVPN_EXPORT int openvpn_plugin_open_v3(
    int version,
    const struct openvpn_plugin_args_open_in *args,
    struct openvpn_plugin_args_open_return *ret)
{
    ret->handle = args->callbacks;
    return OPENVPN_PLUGIN_FUNC_SUCCESS;
}

OPENVPN_EXPORT int openvpn_plugin_func_v3(
    int version,
    const struct openvpn_plugin_args_func_in *args,
    struct openvpn_plugin_args_func_return *ret)
{
    return OPENVPN_PLUGIN_FUNC_ERROR;
}

OPENVPN_EXPORT void openvpn_plugin_close_v1(openvpn_plugin_handle_t handle)
{
}

OPENVPN_EXPORT void *openvpn_plugin_client_constructor_v1(openvpn_plugin_handle_t handle)
{
    struct openvpn_plugin_callbacks *callbacks = handle;

    void *per_client_context = calloc(1, sizeof(int));
    callbacks->plugin_log(PLOG_ERR, PLUGIN_NAME,
        "### openvpn_plugin_client_constructor_v1: %p ###", per_client_context);

    return per_client_context;
}

OPENVPN_EXPORT void openvpn_plugin_client_destructor_v1(
    openvpn_plugin_handle_t handle,
    void *per_client_context)
{
    struct openvpn_plugin_callbacks *callbacks = handle;

    callbacks->plugin_log(PLOG_ERR, PLUGIN_NAME,
        "### openvpn_plugin_client_destructor_v1: %p ###", per_client_context);
    free(per_client_context);
}

OPENVPN_EXPORT int openvpn_plugin_min_version_required_v1()
{
    return 3;
}

Log:

2025-01-23 16:47:15 us=692534 OpenVPN 2.6.13 x86_64-pc-linux-gnu [SSL (OpenSSL)] [LZO] [LZ4] [EPOLL] [PKCS11] [MH/PKTINFO] [AEAD] [DCO]
...
2025-01-23 16:47:15 us=692829 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb579170 ###
...
2025-01-23 16:47:15 us=696927 Initialization Sequence Completed
...
2025-01-23 16:47:27 us=366162 SIGUSR1[hard,] received, process restarting
2025-01-23 16:47:27 us=366207 Restart pause, 1 second(s)
...
2025-01-23 16:47:28 us=367471 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb596700 ###
...
2025-01-23 16:47:36 us=391133 SIGUSR1[hard,] received, process restarting
2025-01-23 16:47:36 us=391172 Restart pause, 1 second(s)
...
2025-01-23 16:47:37 us=392536 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb585640 ###
...
2025-01-23 16:47:37 us=399453 Initialization Sequence Completed
...
^C
...
2025-01-23 16:47:41 us=730149 PLUGIN Issue Demo: ### openvpn_plugin_client_destructor_v1: 0x5776fb585640 ###
...
2025-01-23 16:47:41 us=730235 SIGINT[hard,] received, process exiting

[cron2]

I have never used these constructor/destructor functions myself, but from your description this definitely sounds like something we need to look into. Pinging @dsommers as well.