auth: Changed how auth deinitilization works.

--HG--
branch : HEAD

diff --git a/src/auth/auth-client-connection.c b/src/auth/auth-client-connection.c
index 4c70aedafeed58f92d3bc8a7de6b2bf390a32d9f..cf83820699b9b16e009fd01a473c766132d6e54e 100644 (file)
--- a/src/auth/auth-client-connection.c
+++ b/src/auth/auth-client-connection.c
@@ -319,7 +319,9 @@ auth_client_connection_create(struct auth *auth, int fd, bool login_requests)
        return conn;
 }
 
-void auth_client_connection_destroy(struct auth_client_connection **_conn)
+static void
+auth_client_connection_destroy_full(struct auth_client_connection **_conn,
+                                   bool abort_requests)
 {
         struct auth_client_connection *conn = *_conn;
        struct auth_client_connection *const *clients;
@@ -347,13 +349,21 @@ void auth_client_connection_destroy(struct auth_client_connection **_conn)
        net_disconnect(conn->fd);
        conn->fd = -1;
 
-       if (conn->request_handler != NULL)
-               auth_request_handler_destroy(&conn->request_handler);
+       if (conn->request_handler != NULL) {
+               if (abort_requests)
+                       auth_request_handler_abort_requests(conn->request_handler);
+               auth_request_handler_unref(&conn->request_handler);
+       }
 
         master_service_client_connection_destroyed(master_service);
         auth_client_connection_unref(&conn);
 }
 
+void auth_client_connection_destroy(struct auth_client_connection **_conn)
+{
+       auth_client_connection_destroy_full(_conn, TRUE);
+}
+
 static void auth_client_connection_unref(struct auth_client_connection **_conn)
 {
         struct auth_client_connection *conn = *_conn;
@@ -394,6 +404,6 @@ void auth_client_connections_deinit(void)
 
        clients = array_get_modifiable(&auth_client_connections, &count);
        for (i = count; i > 0; i--)
-               auth_client_connection_destroy(&clients[i-1]);
+               auth_client_connection_destroy_full(&clients[i-1], FALSE);
        array_free(&auth_client_connections);
 }

diff --git a/src/auth/auth-master-connection.c b/src/auth/auth-master-connection.c
index bf16b69346c6de99a02814a7f04393596d0710e6..696142a3634a9a33c02711d6efb9291cde2c5cc3 100644 (file)
--- a/src/auth/auth-master-connection.c
+++ b/src/auth/auth-master-connection.c
@@ -3,7 +3,6 @@
 #include "auth-common.h"
 #include "array.h"
 #include "hash.h"
-#include "llist.h"
 #include "str.h"
 #include "strescape.h"
 #include "str-sanitize.h"
@@ -27,12 +26,6 @@
 #define MAX_INBUF_SIZE 1024
 #define MAX_OUTBUF_SIZE (1024*50)
 
-struct auth_request_list {
-       struct auth_request_list *prev, *next;
-
-       struct auth_request *auth_request;
-};
-
 struct master_userdb_request {
        struct auth_master_connection *conn;
        unsigned int id;
@@ -123,7 +116,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args,
                          const char **error_r)
 {
        struct auth_request *auth_request;
-       struct auth_request_list *request_list;
        const char *const *list, *name, *arg;
        unsigned int id;
 
@@ -140,11 +132,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args,
        auth_request->master = conn;
        auth_master_connection_ref(conn);
 
-       request_list = p_new(auth_request->pool, struct auth_request_list, 1);
-       request_list->auth_request = auth_request;
-       DLLIST_PREPEND(&conn->requests, request_list);
-       auth_request->context = request_list;
-
        if (!auth_request_set_username(auth_request, list[1], error_r)) {
                *request_r = auth_request;
                return 0;
@@ -165,7 +152,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args,
 
        if (auth_request->service == NULL) {
                i_error("BUG: Master sent %s request without service", cmd);
-               DLLIST_REMOVE(&conn->requests, request_list);
                auth_request_unref(&auth_request);
                auth_master_connection_unref(&conn);
                return -1;
@@ -181,7 +167,6 @@ user_callback(enum userdb_result result,
              struct auth_request *auth_request)
 {
        struct auth_master_connection *conn = auth_request->master;
-       struct auth_request_list *list = auth_request->context;
        struct auth_stream_reply *reply = auth_request->userdb_reply;
        string_t *str;
        const char *value;
@@ -214,7 +199,6 @@ user_callback(enum userdb_result result,
        str_append_c(str, '\n');
        (void)o_stream_send(conn->output, str_data(str), str_len(str));
 
-       DLLIST_REMOVE(&conn->requests, list);
        auth_request_unref(&auth_request);
        auth_master_connection_unref(&conn);
 }
@@ -247,7 +231,6 @@ pass_callback(enum passdb_result result,
              struct auth_request *auth_request)
 {
        struct auth_master_connection *conn = auth_request->master;
-       struct auth_request_list *list = auth_request->context;
        struct auth_stream_reply *reply = auth_request->extra_fields;
        string_t *str;
 
@@ -276,7 +259,6 @@ pass_callback(enum passdb_result result,
        str_append_c(str, '\n');
        (void)o_stream_send(conn->output, str_data(str), str_len(str));
 
-       DLLIST_REMOVE(&conn->requests, list);
        auth_request_unref(&auth_request);
        auth_master_connection_unref(&conn);
 }
@@ -532,7 +514,6 @@ void auth_master_connection_destroy(struct auth_master_connection **_conn)
 {
         struct auth_master_connection *conn = *_conn;
         struct auth_master_connection *const *masters;
-       struct auth_request_list *list;
        unsigned int idx;
 
        *_conn = NULL;
@@ -540,9 +521,6 @@ void auth_master_connection_destroy(struct auth_master_connection **_conn)
                return;
        conn->destroyed = TRUE;
 
-       for (list = conn->requests; list != NULL; list = list->next)
-               list->auth_request->destroyed = TRUE;
-
        array_foreach(&auth_master_connections, masters) {
                if (*masters == conn) {
                        idx = array_foreach_idx(&auth_master_connections,

diff --git a/src/auth/auth-request-handler.c b/src/auth/auth-request-handler.c
index 8cf62d115a8a8036f258872ee3acb32eaed9e607..4f57f725560ecf5015eaac6de07efd502d12499c 100644 (file)
--- a/src/auth/auth-request-handler.c
+++ b/src/auth/auth-request-handler.c
@@ -29,8 +29,6 @@ struct auth_request_handler {
        void *context;
 
        auth_request_callback_t *master_callback;
-
-       unsigned int destroyed:1;
 };
 
 static ARRAY_DEFINE(auth_failures_arr, struct auth_request *);
@@ -59,60 +57,38 @@ auth_request_handler_create(auth_request_callback_t *callback, void *context,
        return handler;
 }
 
-static void auth_request_handler_unref(struct auth_request_handler **_handler)
+void auth_request_handler_abort_requests(struct auth_request_handler *handler)
 {
-        struct auth_request_handler *handler = *_handler;
        struct hash_iterate_context *iter;
        void *key, *value;
 
-       *_handler = NULL;
-       i_assert(handler->refcount > 0);
-       if (--handler->refcount > 0)
-               return;
-
        iter = hash_table_iterate_init(handler->requests);
        while (hash_table_iterate(iter, &key, &value)) {
                struct auth_request *auth_request = value;
 
-               auth_request->destroyed = TRUE;
                auth_request_unref(&auth_request);
        }
        hash_table_iterate_deinit(&iter);
        hash_table_clear(handler->requests, TRUE);
-
-       /* notify parent that we're done with all requests */
-       handler->callback(NULL, handler->context);
-
-       hash_table_destroy(&handler->requests);
-       pool_unref(&handler->pool);
 }
 
-static void
-auth_request_handler_destroy_requests(struct auth_request_handler *handler)
-{
-       struct hash_iterate_context *iter;
-       void *key, *value;
-
-       iter = hash_table_iterate_init(handler->requests);
-       while (hash_table_iterate(iter, &key, &value)) {
-               struct auth_request *auth_request = value;
-
-               auth_request->destroyed = TRUE;
-       }
-       hash_table_iterate_deinit(&iter);
-}
-
-void auth_request_handler_destroy(struct auth_request_handler **_handler)
+void auth_request_handler_unref(struct auth_request_handler **_handler)
 {
         struct auth_request_handler *handler = *_handler;
 
        *_handler = NULL;
 
-       i_assert(!handler->destroyed);
+       i_assert(handler->refcount > 0);
+       if (--handler->refcount > 0)
+               return;
+
+       i_assert(hash_table_count(handler->requests) == 0);
+
+       /* notify parent that we're done with all requests */
+       handler->callback(NULL, handler->context);
 
-       handler->destroyed = TRUE;
-       auth_request_handler_destroy_requests(handler);
-       auth_request_handler_unref(&handler);
+       hash_table_destroy(&handler->requests);
+       pool_unref(&handler->pool);
 }
 
 void auth_request_handler_set(struct auth_request_handler *handler,
@@ -198,8 +174,7 @@ auth_request_handle_failure(struct auth_request *request,
        auth_request_handler_remove(handler, request);
 
        if (request->no_failure_delay) {
-               /* passdb specifically requested not to delay the
-                  reply. */
+               /* passdb specifically requested not to delay the reply. */
                handler->callback(reply, handler->context);
                auth_request_unref(&request);
                return;

diff --git a/src/auth/auth-request-handler.h b/src/auth/auth-request-handler.h
index 834c311fad940d23644be52542cb6ed6779ad97b..e9bc847fa6660175da7d4a0b35ffcbbd2d785901 100644 (file)
--- a/src/auth/auth-request-handler.h
+++ b/src/auth/auth-request-handler.h
@@ -29,7 +29,8 @@ auth_request_handler_create(auth_request_callback_t *callback, void *context,
                (auth_request_callback_t *)callback, context, \
                master_callback)
 #endif
-void auth_request_handler_destroy(struct auth_request_handler **handler);
+void auth_request_handler_unref(struct auth_request_handler **handler);
+void auth_request_handler_abort_requests(struct auth_request_handler *handler);
 
 void auth_request_handler_set(struct auth_request_handler *handler,
                              unsigned int connect_uid,

diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c
index 7c922da0ca9b9ed9e5bc7061a56cb87a5c7d0e75..b8fdebf8582a35c1e06c6db4777c6cab3f146fc5 100644 (file)
--- a/src/auth/auth-request.c
+++ b/src/auth/auth-request.c
@@ -389,13 +389,6 @@ auth_request_handle_passdb_callback(enum passdb_result *result,
                            strlen(request->passdb_password));
        }
 
-       if (request->destroyed) {
-               /* the passdb may have been freed already. this request won't
-                  be sent anywhere anyway, so just fail it immediately. */
-               *result = PASSDB_RESULT_INTERNAL_FAILURE;
-               return TRUE;
-       }
-
        if (request->passdb->set->deny &&
            *result != PASSDB_RESULT_USER_UNKNOWN) {
                /* deny passdb. we can get through this step only if the
@@ -559,9 +552,14 @@ void auth_request_verify_plain(struct auth_request *request,
        auth_request_set_state(request, AUTH_REQUEST_STATE_PASSDB);
        request->credentials_scheme = NULL;
 
-       if (passdb->blocking)
+       if (passdb->iface.verify_plain == NULL) {
+               /* we're deinitializing and just want to get rid of this
+                  request */
+               auth_request_verify_plain_callback(
+                       PASSDB_RESULT_INTERNAL_FAILURE, request);
+       } else if (passdb->blocking) {
                passdb_blocking_verify_plain(request);
-       else {
+       } else if (passdb->iface.verify_plain != NULL) {
                passdb->iface.verify_plain(request, password,
                                           auth_request_verify_plain_callback);
        }
@@ -741,14 +739,6 @@ void auth_request_userdb_callback(enum userdb_result result,
 {
        struct userdb_module *userdb = request->userdb->userdb;
 
-       if (request->destroyed) {
-               /* the userdb may have been freed already. this request won't
-                  be sent anywhere anyway, so just fail it immediately. */
-               request->private_callback.
-                       userdb(USERDB_RESULT_INTERNAL_FAILURE, request);
-               return;
-       }
-
        if (result != USERDB_RESULT_OK && request->userdb->next != NULL) {
                /* try next userdb. */
                if (result == USERDB_RESULT_INTERNAL_FAILURE)
@@ -823,7 +813,11 @@ void auth_request_lookup_user(struct auth_request *request,
                }
        }
 
-       if (userdb->blocking)
+       if (userdb->iface->lookup == NULL) {
+               /* we are deinitializing */
+               auth_request_userdb_callback(USERDB_RESULT_INTERNAL_FAILURE,
+                                            request);
+       } else if (userdb->blocking)
                userdb_blocking_lookup(request);
        else
                userdb->iface->lookup(request, auth_request_userdb_callback);

diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h
index a08708e11a9173c8611964d4055ec0fa6cf4dbc7..3e7bc47285f81402430c0a44923e981dc8276ede 100644 (file)
--- a/src/auth/auth-request.h
+++ b/src/auth/auth-request.h
@@ -109,7 +109,6 @@ struct auth_request {
        unsigned int userdb_lookup:1;
        unsigned int userdb_lookup_failed:1;
        unsigned int secured:1;
-       unsigned int destroyed:1;
 
        /* ... mechanism specific data ... */
 };

diff --git a/src/auth/auth.c b/src/auth/auth.c
index e8175cc4585602deb36f4f4210c07c6717d700a4..3b6cb039c2602c2cc6275766f084349c32de53e0 100644 (file)
--- a/src/auth/auth.c
+++ b/src/auth/auth.c
@@ -47,7 +47,7 @@ auth_userdb_preinit(struct auth *auth, const struct auth_userdb_settings *set)
                userdb_preinit(auth->pool, set->driver, set->args);
 }
 
-struct auth *
+static struct auth *
 auth_preinit(const struct auth_settings *set, const char *service, pool_t pool,
             const struct mechanisms_register *reg)
 {
@@ -185,7 +185,7 @@ static void auth_mech_list_verify_passdb(struct auth *auth)
        }
 }
 
-void auth_init(struct auth *auth)
+static void auth_init(struct auth *auth)
 {
        struct auth_passdb *passdb;
        struct auth_userdb *userdb;
@@ -200,22 +200,17 @@ void auth_init(struct auth *auth)
        auth_mech_list_verify_passdb(auth);
 }
 
-void auth_deinit(struct auth **_auth)
+static void auth_deinit(struct auth *auth)
 {
-        struct auth *auth = *_auth;
        struct auth_passdb *passdb;
        struct auth_userdb *userdb;
 
-       *_auth = NULL;
-
        for (passdb = auth->masterdbs; passdb != NULL; passdb = passdb->next)
                passdb_deinit(passdb->passdb);
        for (passdb = auth->passdbs; passdb != NULL; passdb = passdb->next)
                passdb_deinit(passdb->passdb);
        for (userdb = auth->userdbs; userdb != NULL; userdb = userdb->next)
                userdb_deinit(userdb->userdb);
-
-       pool_unref(&auth->pool);
 }
 
 struct auth *auth_find_service(const char *name)
@@ -279,6 +274,14 @@ void auths_init(void)
 }
 
 void auths_deinit(void)
+{
+       struct auth *const *auth;
+
+       array_foreach(&auths, auth)
+               auth_deinit(*auth);
+}
+
+void auths_free(void)
 {
        struct auth **auth;
        unsigned int i, count;
@@ -287,6 +290,6 @@ void auths_deinit(void)
           the first auth pool that used them */
        auth = array_get_modifiable(&auths, &count);
        for (i = count; i > 0; i--)
-               auth_deinit(&auth[i-1]);
+               pool_unref(&auth[i-1]->pool);
        array_free(&auths);
 }

diff --git a/src/auth/auth.h b/src/auth/auth.h
index 2f8ab37b0ee96a2dd1ed27e05af9d97c5464effe..4c96c082d77e8e3d68962c56bf1abac585a9803e 100644 (file)
--- a/src/auth/auth.h
+++ b/src/auth/auth.h
@@ -32,12 +32,6 @@ struct auth {
 
 extern struct auth_penalty *auth_penalty;
 
-struct auth *
-auth_preinit(const struct auth_settings *set, const char *service, pool_t pool,
-            const struct mechanisms_register *mech_reg);
-void auth_init(struct auth *auth);
-void auth_deinit(struct auth **auth);
-
 struct auth *auth_find_service(const char *name);
 
 void auths_preinit(const struct auth_settings *set, pool_t pool,
@@ -45,5 +39,6 @@ void auths_preinit(const struct auth_settings *set, pool_t pool,
                   const char *const *services);
 void auths_init(void);
 void auths_deinit(void);
+void auths_free(void);
 
 #endif

diff --git a/src/auth/main.c b/src/auth/main.c
index eed2bb6d2ad546bb9e46894da2d8e10813925f71..ad538cc2c7fa58ecbe6a6c2f96ffa9487059f5d7 100644 (file)
--- a/src/auth/main.c
+++ b/src/auth/main.c
@@ -151,23 +151,20 @@ static void main_init(void)
 
 static void main_deinit(void)
 {
-       if (auth_worker_client != NULL)
-               auth_worker_client_destroy(&auth_worker_client);
-       else
-               auth_request_handler_flush_failures(TRUE);
+       /* deinit auth workers, which aborts pending requests */
+        auth_worker_server_deinit();
+       /* deinit passdbs and userdbs. it aborts any pending async requests. */
+       auths_deinit();
+       /* flush pending requests */
+       auth_request_handler_deinit();
+       /* there are no more auth requests */
+       auths_free();
 
        auth_client_connections_deinit();
        auth_master_connections_deinit();
-        auth_worker_server_deinit();
 
-       auth_request_handler_deinit();
-       /* there may still be some async auth requests left, but above
-          functions should have marked all of them as destroyed. pass/userdb
-          deinits should abort the pending requests, which still triggers
-          callbacks, which should avoid crashes by checking the destroyed
-          state. */
-       auths_deinit();
-       passdb_cache_deinit();
+       if (auth_worker_client != NULL)
+               auth_worker_client_destroy(&auth_worker_client);
 
        mech_register_deinit(&mech_reg);
        mech_deinit(global_auth_settings);
@@ -179,6 +176,7 @@ static void main_deinit(void)
 
        userdbs_deinit();
        passdbs_deinit();
+       passdb_cache_deinit();
         password_schemes_deinit();
        sql_drivers_deinit();
        random_deinit();

diff --git a/src/auth/passdb.c b/src/auth/passdb.c
index 7be58eeceb887cf01a849ca4b817519484747040..a936e1eef0761bb7a837253cbabe1dd91c29b378 100644 (file)
--- a/src/auth/passdb.c
+++ b/src/auth/passdb.c
@@ -11,6 +11,10 @@
 static ARRAY_DEFINE(passdb_interfaces, struct passdb_module_interface *);
 static ARRAY_DEFINE(passdb_modules, struct passdb_module *);
 
+static const struct passdb_module_interface passdb_iface_deinit = {
+       .name = "deinit"
+};
+
 static struct passdb_module_interface *passdb_interface_find(const char *name)
 {
        struct passdb_module_interface *const *ifaces;
@@ -230,6 +234,9 @@ void passdb_deinit(struct passdb_module *passdb)
 
        if (passdb->iface.deinit != NULL)
                passdb->iface.deinit(passdb);
+
+       /* make sure passdb isn't accessed again */
+       passdb->iface = passdb_iface_deinit;
 }
 
 void passdbs_generate_md5(unsigned char md5[MD5_RESULTLEN])

diff --git a/src/auth/userdb.c b/src/auth/userdb.c
index 2fa1f3a5cca86920e58826577bf969fd7d859634..5563dd96b64b11309d6a1edfe01e8e906430f39c 100644 (file)
--- a/src/auth/userdb.c
+++ b/src/auth/userdb.c
@@ -12,6 +12,10 @@
 static ARRAY_DEFINE(userdb_interfaces, struct userdb_module_interface *);
 static ARRAY_DEFINE(userdb_modules, struct userdb_module *);
 
+static const struct userdb_module_interface userdb_iface_deinit = {
+       .name = "deinit"
+};
+
 static struct userdb_module_interface *userdb_interface_find(const char *name)
 {
        struct userdb_module_interface *const *ifaces;
@@ -171,6 +175,9 @@ void userdb_deinit(struct userdb_module *userdb)
 
        if (userdb->iface->deinit != NULL)
                userdb->iface->deinit(userdb);
+
+       /* make sure userdb isn't accessed again */
+       userdb->iface = &userdb_iface_deinit;
 }
 
 void userdbs_generate_md5(unsigned char md5[MD5_RESULTLEN])