diff options
| -rw-r--r-- | src/analysis/loading.c | 1 | ||||
| -rw-r--r-- | src/analysis/project.c | 103 |
2 files changed, 77 insertions, 27 deletions
diff --git a/src/analysis/loading.c b/src/analysis/loading.c index fa2118f..b8fbeea 100644 --- a/src/analysis/loading.c +++ b/src/analysis/loading.c @@ -765,6 +765,7 @@ void g_content_explorer_delete_group(GContentExplorer *explorer, wgroup_id_t wid g_mutex_lock(&explorer->mutex); group = g_content_explorer_find_group(explorer, wid); + assert(group != NULL); /* Supression des contenus chargés */ diff --git a/src/analysis/project.c b/src/analysis/project.c index 83b41c4..66faa16 100644 --- a/src/analysis/project.c +++ b/src/analysis/project.c @@ -853,33 +853,38 @@ static void g_loading_handler_dispose(GLoadingHandler *handler) GContentResolver *resolver; /* Resolveur de contenus */ size_t i; /* Boucle de parcours */ - /* Supression des groupes de travail */ + /** + * On se sert du projet comme sentinelle pour la validité des + * identifiants handler->exp_wids[i]. + */ - explorer = get_current_content_explorer(); - resolver = get_current_content_resolver(); + if (handler->project != NULL) + { + /* Supression des groupes de travail */ - g_signal_handlers_disconnect_by_func(explorer, G_CALLBACK(on_new_content_explored), handler); - g_signal_handlers_disconnect_by_func(resolver, G_CALLBACK(on_new_content_resolved), handler); + explorer = get_current_content_explorer(); + resolver = get_current_content_resolver(); - g_mutex_lock(&handler->mutex); + g_signal_handlers_disconnect_by_func(explorer, G_CALLBACK(on_new_content_explored), handler); + g_signal_handlers_disconnect_by_func(resolver, G_CALLBACK(on_new_content_resolved), handler); - for (i = 0; i < handler->exp_count; i++) - { - g_content_resolver_delete_group(resolver, handler->exp_wids[i]); - g_content_explorer_delete_group(explorer, handler->exp_wids[i]); - } + for (i = 0; i < handler->exp_count; i++) + { + g_content_resolver_delete_group(resolver, handler->exp_wids[i]); + g_content_explorer_delete_group(explorer, handler->exp_wids[i]); + } - g_mutex_unlock(&handler->mutex); + g_object_unref(G_OBJECT(explorer)); + g_object_unref(G_OBJECT(resolver)); - g_object_unref(G_OBJECT(explorer)); - g_object_unref(G_OBJECT(resolver)); + /* Nettoyage plus général */ - /* Nettoyage plus général */ + g_mutex_clear(&handler->mutex); + g_cond_clear(&handler->wait_cond); - g_mutex_clear(&handler->mutex); - g_cond_clear(&handler->wait_cond); + g_clear_object(&handler->project); - g_object_unref(G_OBJECT(handler->project)); + } G_OBJECT_CLASS(g_loading_handler_parent_class)->dispose(G_OBJECT(handler)); @@ -1089,13 +1094,61 @@ static bool g_loading_handler_check(GLoadingHandler *handler, wgroup_id_t wid) bool result; /* Bilan à retourner */ size_t i; /* Boucle de parcours */ - assert(!g_mutex_trylock(&handler->mutex)); + /** + * Les deux appelants on_new_content_explored() et on_new_content_resolved() + * ne doivent absolument pas poser un large verrou en filtrant les identifiants + * via cette fonction. + * + * On pose donc ce verrou ici. + * + * La raison mineure est que les deux appelants n'accèdent qu'en lecture + * (à peu de chose près) aux propriétés de l'objet handler. + * + * La raison principale est la suivante : + * + * - on_new_content_explored() crée un groupe de tâches via l'appel + * g_content_resolver_create_group(). + * + * - on_new_content_resolved() peut conduire à une libération de l'objet + * handler, procédure qui va libérer ensuite le groupe de tâches associé. + * + * Ce qui peut conduire à un dead lock avec un large verrou : + * + * - un thread T1 termine une résolution wid=3 ; il va donc appeler + * tous les handlers via le signal "resolved". + * + * - T1 va rencontrer le handler qui gère wid=3. C'était la dernière + * résolution, donc un broadcast sur le compteur "resolved" va + * être émis. + * + * - un thread T2 en attente dans g_loading_handler_process() va donc + * terminer sa tâche. Depuis la fonction g_loading_handler_dispose(), + * cette tâche va libérer le groupe associé, dont l'exécution est + * assurée par T1. + * + * - le thread T2 va donc terminer sur un g_thread_join() dans la fonction + * g_work_group_dispose(), en attandant que T1 remarque l'ordre d'arrêt. + * + * - or T1 va continuer la propagation du signal "resolved" aux autres + * résolveurs (par exemple, celui gérant wid=4). + * + * - nouvelle exécution de on_new_content_resolved(), qui bloque cette + * fois, car le handler wid=4 est occupé dans un thread T3 à la fonction + * on_new_content_explored(), suite à un signal "explored" avec wid=4. + * + * - si le verrou handler->mutex est posé en même temps que la modification + * des groupes de tâches, alors T1, T2 et T3 vont se bloquer mutuellement. + */ + + g_mutex_lock(&handler->mutex); result = false; for (i = 0; i < handler->exp_count && !result; i++) result = (handler->exp_wids[i] == wid); + g_mutex_unlock(&handler->mutex); + return result; } @@ -1122,8 +1175,6 @@ static void on_new_content_explored(GContentExplorer *explorer, wgroup_id_t wid, GContentResolver *resolver; /* Resolveur de contenus */ size_t i; /* Boucle de parcours */ - g_mutex_lock(&handler->mutex); - if (g_loading_handler_check(handler, wid)) { available = g_content_explorer_get_all(explorer, wid, &count); @@ -1142,8 +1193,6 @@ static void on_new_content_explored(GContentExplorer *explorer, wgroup_id_t wid, } - g_mutex_unlock(&handler->mutex); - } @@ -1173,8 +1222,6 @@ static void on_new_content_resolved(GContentResolver *resolver, wgroup_id_t wid, xmlXPathObjectPtr xobject; /* Cible d'une recherche */ bool status; /* Bilan d'une restauration */ - g_mutex_lock(&handler->mutex); - if (g_loading_handler_check(handler, wid)) { available = g_content_resolver_get_all(resolver, wid, &count); @@ -1268,13 +1315,15 @@ static void on_new_content_resolved(GContentResolver *resolver, wgroup_id_t wid, /* Si c'était la dernière résolution... */ + g_mutex_lock(&handler->mutex); + handler->resolved++; g_cond_broadcast(&handler->wait_cond); - } + g_mutex_unlock(&handler->mutex); - g_mutex_unlock(&handler->mutex); + } } |