persistent_client_expiration does not work with logrotate/reload

[ckrey]

This problem is in MQTTv311:

When log is rotated and mosquitto gets reloaded, inactive clients are detected as disconnected. loop.coutputs the following message:

/var/log/mosquitto/mosquitto.log

...
1573532162: Client rkix disconnected.
...

loop.c calls context__disconnect which calls session_expiry__add which
sets the expiry for the inactive session to now + persistent_client_expiration.

If persistent_client_expiration is longer than one day, those clients will never expire...

/etc/mosquitto/mosquitto.conf

...
persistent_client_expiration 1w
...

/etc/logrotate.d/mosquitto

/var/log/mosquitto/mosquitto.log {
        rotate 7
        daily
        compress
        size 100k
        nocreate
        missingok
        postrotate
                if invoke-rc.d mosquitto status > /dev/null 2>&1; then \
                        invoke-rc.d mosquitto reload > /dev/null 2>&1; \
                fi;
        endscript
}

[ckrey]

As a result those inactive clients and possible a large number of queued messages are never purged from mosquitto (and mosquitto.db)

[ralight]

Other people have seen what looks like clients being disconnected on HUP, but I've never been able to reproduce it. Do you have any other mosquitto config that might be different to what I have?

[ckrey]

I cannot reproduce the problem of clients being disconnected on HUP using a stripped down test environment.

What I can reproduce is the fact that v311 clients never expire because for v311 clients session_expiry_interval is always set to UINT32_MAX on connect and the expiration check does
check for this value.

Probably a fix for the problem would be:

diff --git a/src/session_expiry.c b/src/session_expiry.c
index e720b703..7e182126 100644
--- a/src/session_expiry.c
+++ b/src/session_expiry.c
@@ -44,7 +44,7 @@ int session_expiry__add(struct mosquitto_db *db, struct mosquitto *context)
        item->context = context;
        item->context->session_expiry_time = time(NULL);
        if(db->config->persistent_client_expiration == 0 ||
-                       db->config->persistent_client_expiration < item->context->session_expiry_interval){
+                       db->config->persistent_client_expiration > item->context->session_expiry_interval){

                item->context->session_expiry_time += item->context->session_expiry_interval;
        }else{
@@ -95,8 +95,7 @@ void session_expiry__check(struct mosquitto_db *db, time_t now)
        last_check = now;

        DL_FOREACH_SAFE(expiry_list, item, tmp){
-               if(item->context->session_expiry_interval != UINT32_MAX
-                               && item->context->session_expiry_time < now){
+               if(item->context->session_expiry_time < now){

                        context = item->context;
                        session_expiry__remove(context);

P.S.
session_expiry__check does actually do the expiry, G_CLIENTS_EXPIRED_INC() in loop.c is never executed. You may want to include the stats and logging into session_expiry.c

[ralight]

Good point about the UINT32_MAX value. We have to keep that as a special value though which means "never expire" - except that the custom expiration must override it.