Commit 541966f3 authored by Andrea Azzarone's avatar Andrea Azzarone 🚴 Committed by Jordi Mas

metadata-manager: Remove singleton

GeditDocument may attempt to use GeditMetadataManager after GeditApplication has
already called gedit_data_manager_shutdown. This happens because right now
_gedit_tab_load is not cancelled when the tab is closed, but it's only cancelled
if the user dismisses the loading operation from the infobar.

Also cancelling _gedit_tab_load is not enough because GtkSourceFileLoader
increases the reference count of the source buffer (in the case the
GeditDocument) until the async operation is completed or cancelled, hence
GeditDocument will attempt to use GeditMetadataManager in gedit_document_dispose
after GeditApplication has already called gedit_data_manager_shutdown.

To properly solve this - and avoid similar issues - make GeditMetadataManager
reference countable and keep an hard reference in each GeditDocument and in the
GeditApp.

Closes: #145
parent ba2da3e2
Pipeline #75797 passed with stage
in 6 minutes and 49 seconds
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#define GEDIT_APP_PRIVATE_H #define GEDIT_APP_PRIVATE_H
#include "gedit-app.h" #include "gedit-app.h"
#include "gedit-metadata-manager.h"
#include "gedit-settings.h" #include "gedit-settings.h"
#include "gedit-menu-extension.h" #include "gedit-menu-extension.h"
...@@ -48,6 +49,8 @@ GtkPrintSettings *_gedit_app_get_default_print_settings (GeditApp *app); ...@@ -48,6 +49,8 @@ GtkPrintSettings *_gedit_app_get_default_print_settings (GeditApp *app);
void _gedit_app_set_default_print_settings (GeditApp *app, void _gedit_app_set_default_print_settings (GeditApp *app,
GtkPrintSettings *settings); GtkPrintSettings *settings);
GeditMetadataManager *_gedit_app_get_metadata_manager (GeditApp *app);
GeditSettings *_gedit_app_get_settings (GeditApp *app); GeditSettings *_gedit_app_get_settings (GeditApp *app);
GMenuModel *_gedit_app_get_hamburger_menu (GeditApp *app); GMenuModel *_gedit_app_get_hamburger_menu (GeditApp *app);
......
...@@ -63,6 +63,10 @@ typedef struct ...@@ -63,6 +63,10 @@ typedef struct
{ {
GeditPluginsEngine *engine; GeditPluginsEngine *engine;
#ifndef ENABLE_GVFS_METADATA
GeditMetadataManager *metadata_manager;
#endif
GtkCssProvider *theme_provider; GtkCssProvider *theme_provider;
GeditLockdownMask lockdown; GeditLockdownMask lockdown;
...@@ -170,6 +174,10 @@ gedit_app_dispose (GObject *object) ...@@ -170,6 +174,10 @@ gedit_app_dispose (GObject *object)
priv = gedit_app_get_instance_private (GEDIT_APP (object)); priv = gedit_app_get_instance_private (GEDIT_APP (object));
#ifndef ENABLE_GVFS_METADATA
g_clear_object (&priv->metadata_manager);
#endif
g_clear_object (&priv->ui_settings); g_clear_object (&priv->ui_settings);
g_clear_object (&priv->window_settings); g_clear_object (&priv->window_settings);
g_clear_object (&priv->settings); g_clear_object (&priv->settings);
...@@ -774,7 +782,7 @@ gedit_app_startup (GApplication *application) ...@@ -774,7 +782,7 @@ gedit_app_startup (GApplication *application)
#ifndef ENABLE_GVFS_METADATA #ifndef ENABLE_GVFS_METADATA
cache_dir = gedit_dirs_get_user_cache_dir (); cache_dir = gedit_dirs_get_user_cache_dir ();
metadata_filename = g_build_filename (cache_dir, "gedit-metadata.xml", NULL); metadata_filename = g_build_filename (cache_dir, "gedit-metadata.xml", NULL);
gedit_metadata_manager_init (metadata_filename); priv->metadata_manager = gedit_metadata_manager_new (metadata_filename);
g_free (metadata_filename); g_free (metadata_filename);
#endif #endif
...@@ -1246,10 +1254,6 @@ gedit_app_shutdown (GApplication *app) ...@@ -1246,10 +1254,6 @@ gedit_app_shutdown (GApplication *app)
*/ */
G_APPLICATION_CLASS (gedit_app_parent_class)->shutdown (app); G_APPLICATION_CLASS (gedit_app_parent_class)->shutdown (app);
#ifndef ENABLE_GVFS_METADATA
gedit_metadata_manager_shutdown ();
#endif
gedit_dirs_shutdown (); gedit_dirs_shutdown ();
} }
...@@ -1871,6 +1875,24 @@ _gedit_app_get_settings (GeditApp *app) ...@@ -1871,6 +1875,24 @@ _gedit_app_get_settings (GeditApp *app)
return priv->settings; return priv->settings;
} }
GeditMetadataManager *
_gedit_app_get_metadata_manager (GeditApp *app)
{
#ifndef ENABLE_GVFS_METADATA
GeditAppPrivate *priv;
g_return_val_if_fail (GEDIT_IS_APP (app), NULL);
priv = gedit_app_get_instance_private (app);
return priv->metadata_manager;
#else
g_assert_not_reached ();
return NULL;
#endif
}
GMenuModel * GMenuModel *
_gedit_app_get_hamburger_menu (GeditApp *app) _gedit_app_get_hamburger_menu (GeditApp *app)
{ {
......
...@@ -31,6 +31,8 @@ ...@@ -31,6 +31,8 @@
#include <string.h> #include <string.h>
#include <glib/gi18n.h> #include <glib/gi18n.h>
#include "gedit-app.h"
#include "gedit-app-private.h"
#include "gedit-settings.h" #include "gedit-settings.h"
#include "gedit-debug.h" #include "gedit-debug.h"
#include "gedit-utils.h" #include "gedit-utils.h"
...@@ -67,6 +69,8 @@ typedef struct ...@@ -67,6 +69,8 @@ typedef struct
*/ */
GtkSourceSearchContext *search_context; GtkSourceSearchContext *search_context;
GeditMetadataManager *metadata_manager;
guint user_action; guint user_action;
guint language_set_by_user : 1; guint language_set_by_user : 1;
...@@ -214,6 +218,7 @@ gedit_document_dispose (GObject *object) ...@@ -214,6 +218,7 @@ gedit_document_dispose (GObject *object)
g_clear_object (&priv->editor_settings); g_clear_object (&priv->editor_settings);
g_clear_object (&priv->metadata_info); g_clear_object (&priv->metadata_info);
g_clear_object (&priv->search_context); g_clear_object (&priv->search_context);
g_clear_object (&priv->metadata_manager);
G_OBJECT_CLASS (gedit_document_parent_class)->dispose (object); G_OBJECT_CLASS (gedit_document_parent_class)->dispose (object);
} }
...@@ -379,6 +384,15 @@ gedit_document_constructed (GObject *object) ...@@ -379,6 +384,15 @@ gedit_document_constructed (GObject *object)
priv = gedit_document_get_instance_private (doc); priv = gedit_document_get_instance_private (doc);
if (!priv->use_gvfs_metadata)
{
GeditMetadataManager *metadata_manager;
metadata_manager = _gedit_app_get_metadata_manager (GEDIT_APP (g_application_get_default ()));
g_assert (GEDIT_IS_METADATA_MANAGER (metadata_manager));
priv->metadata_manager = g_object_ref (metadata_manager);
}
/* Bind construct properties. */ /* Bind construct properties. */
g_settings_bind (priv->editor_settings, g_settings_bind (priv->editor_settings,
GEDIT_SETTINGS_ENSURE_TRAILING_NEWLINE, GEDIT_SETTINGS_ENSURE_TRAILING_NEWLINE,
...@@ -1566,7 +1580,7 @@ get_metadata_from_metadata_manager (GeditDocument *doc, ...@@ -1566,7 +1580,7 @@ get_metadata_from_metadata_manager (GeditDocument *doc,
if (location != NULL) if (location != NULL)
{ {
return gedit_metadata_manager_get (location, key); return gedit_metadata_manager_get (priv->metadata_manager, location, key);
} }
return NULL; return NULL;
...@@ -1694,7 +1708,7 @@ gedit_document_set_metadata (GeditDocument *doc, ...@@ -1694,7 +1708,7 @@ gedit_document_set_metadata (GeditDocument *doc,
} }
else else
{ {
gedit_metadata_manager_set (location, key, value); gedit_metadata_manager_set (priv->metadata_manager, location, key, value);
} }
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
* This file is part of gedit * This file is part of gedit
* *
* Copyright (C) 2003-2007 Paolo Maggi * Copyright (C) 2003-2007 Paolo Maggi
* Copyright (C) 2019 Canonical LTD
* *
* This program is free software; you can redistribute it and/or modify * This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by * it under the terms of the GNU General Public License as published by
...@@ -29,8 +30,6 @@ ...@@ -29,8 +30,6 @@
#define MAX_ITEMS 50 #define MAX_ITEMS 50
typedef struct _GeditMetadataManager GeditMetadataManager;
typedef struct _Item Item; typedef struct _Item Item;
struct _Item struct _Item
...@@ -43,6 +42,8 @@ struct _Item ...@@ -43,6 +42,8 @@ struct _Item
struct _GeditMetadataManager struct _GeditMetadataManager
{ {
GObject parent_instance;
/* It is true if the file has been read. */ /* It is true if the file has been read. */
gboolean values_loaded; gboolean values_loaded;
...@@ -53,9 +54,18 @@ struct _GeditMetadataManager ...@@ -53,9 +54,18 @@ struct _GeditMetadataManager
gchar *metadata_filename; gchar *metadata_filename;
}; };
static gboolean gedit_metadata_manager_save (gpointer data); enum
{
PROP_0,
PROP_METADATA_FILENAME,
LAST_PROP
};
static GeditMetadataManager *gedit_metadata_manager = NULL; static GParamSpec *properties[LAST_PROP];
G_DEFINE_TYPE (GeditMetadataManager, gedit_metadata_manager, G_TYPE_OBJECT);
static gboolean gedit_metadata_manager_save (GeditMetadataManager *self);
static void static void
item_free (gpointer data) item_free (gpointer data)
...@@ -77,81 +87,23 @@ item_free (gpointer data) ...@@ -77,81 +87,23 @@ item_free (gpointer data)
} }
static void static void
gedit_metadata_manager_arm_timeout (void) gedit_metadata_manager_arm_timeout (GeditMetadataManager *self)
{ {
if (gedit_metadata_manager->timeout_id == 0) if (self->timeout_id == 0)
{ {
gedit_metadata_manager->timeout_id = self->timeout_id =
g_timeout_add_seconds_full (G_PRIORITY_DEFAULT_IDLE, g_timeout_add_seconds_full (G_PRIORITY_DEFAULT_IDLE,
2, 2,
(GSourceFunc)gedit_metadata_manager_save, (GSourceFunc)gedit_metadata_manager_save,
NULL, self,
NULL); NULL);
} }
} }
/**
* gedit_metadata_manager_init:
* @metadata_filename: the filename where the metadata is stored.
*
* This function initializes the metadata manager.
* See also gedit_metadata_manager_shutdown().
*/
void
gedit_metadata_manager_init (const gchar *metadata_filename)
{
gedit_debug (DEBUG_METADATA);
if (gedit_metadata_manager != NULL)
{
return;
}
gedit_metadata_manager = g_new0 (GeditMetadataManager, 1);
gedit_metadata_manager->values_loaded = FALSE;
gedit_metadata_manager->items =
g_hash_table_new_full (g_str_hash,
g_str_equal,
g_free,
item_free);
gedit_metadata_manager->metadata_filename = g_strdup (metadata_filename);
}
/**
* gedit_metadata_manager_shutdown:
*
* This function frees the internal data of the metadata manager.
* See also gedit_metadata_manager_init().
*/
void
gedit_metadata_manager_shutdown (void)
{
gedit_debug (DEBUG_METADATA);
if (gedit_metadata_manager == NULL)
return;
if (gedit_metadata_manager->timeout_id)
{
g_source_remove (gedit_metadata_manager->timeout_id);
gedit_metadata_manager->timeout_id = 0;
gedit_metadata_manager_save (NULL);
}
if (gedit_metadata_manager->items != NULL)
g_hash_table_destroy (gedit_metadata_manager->items);
g_free (gedit_metadata_manager->metadata_filename);
g_free (gedit_metadata_manager);
gedit_metadata_manager = NULL;
}
static void static void
parseItem (xmlDocPtr doc, xmlNodePtr cur) gedit_metadata_manager_parse_item (GeditMetadataManager *self,
xmlDocPtr doc,
xmlNodePtr cur)
{ {
Item *item; Item *item;
...@@ -213,7 +165,7 @@ parseItem (xmlDocPtr doc, xmlNodePtr cur) ...@@ -213,7 +165,7 @@ parseItem (xmlDocPtr doc, xmlNodePtr cur)
cur = cur->next; cur = cur->next;
} }
g_hash_table_insert (gedit_metadata_manager->items, g_hash_table_insert (self->items,
g_strdup ((gchar *)uri), g_strdup ((gchar *)uri),
item); item);
...@@ -223,32 +175,32 @@ parseItem (xmlDocPtr doc, xmlNodePtr cur) ...@@ -223,32 +175,32 @@ parseItem (xmlDocPtr doc, xmlNodePtr cur)
/* Returns FALSE in case of error. */ /* Returns FALSE in case of error. */
static gboolean static gboolean
load_values (void) gedit_metadata_manager_load_values (GeditMetadataManager *self)
{ {
xmlDocPtr doc; xmlDocPtr doc;
xmlNodePtr cur; xmlNodePtr cur;
gedit_debug (DEBUG_METADATA); gedit_debug (DEBUG_METADATA);
g_return_val_if_fail (gedit_metadata_manager != NULL, FALSE); g_return_val_if_fail (self != NULL, FALSE);
g_return_val_if_fail (gedit_metadata_manager->values_loaded == FALSE, FALSE); g_return_val_if_fail (self->values_loaded == FALSE, FALSE);
gedit_metadata_manager->values_loaded = TRUE; self->values_loaded = TRUE;
xmlKeepBlanksDefault (0); xmlKeepBlanksDefault (0);
if (gedit_metadata_manager->metadata_filename == NULL) if (self->metadata_filename == NULL)
{ {
return FALSE; return FALSE;
} }
/* TODO: avoid races */ /* TODO: avoid races */
if (!g_file_test (gedit_metadata_manager->metadata_filename, G_FILE_TEST_EXISTS)) if (!g_file_test (self->metadata_filename, G_FILE_TEST_EXISTS))
{ {
return TRUE; return TRUE;
} }
doc = xmlParseFile (gedit_metadata_manager->metadata_filename); doc = xmlParseFile (self->metadata_filename);
if (doc == NULL) if (doc == NULL)
{ {
...@@ -259,7 +211,7 @@ load_values (void) ...@@ -259,7 +211,7 @@ load_values (void)
if (cur == NULL) if (cur == NULL)
{ {
g_message ("The metadata file '%s' is empty", g_message ("The metadata file '%s' is empty",
g_path_get_basename (gedit_metadata_manager->metadata_filename)); g_path_get_basename (self->metadata_filename));
xmlFreeDoc (doc); xmlFreeDoc (doc);
return TRUE; return TRUE;
...@@ -268,7 +220,7 @@ load_values (void) ...@@ -268,7 +220,7 @@ load_values (void)
if (xmlStrcmp (cur->name, (const xmlChar *) "metadata")) if (xmlStrcmp (cur->name, (const xmlChar *) "metadata"))
{ {
g_message ("File '%s' is of the wrong type", g_message ("File '%s' is of the wrong type",
g_path_get_basename (gedit_metadata_manager->metadata_filename)); g_path_get_basename (self->metadata_filename));
xmlFreeDoc (doc); xmlFreeDoc (doc);
return FALSE; return FALSE;
...@@ -279,7 +231,7 @@ load_values (void) ...@@ -279,7 +231,7 @@ load_values (void)
while (cur != NULL) while (cur != NULL)
{ {
parseItem (doc, cur); gedit_metadata_manager_parse_item (self, doc, cur);
cur = cur->next; cur = cur->next;
} }
...@@ -291,19 +243,22 @@ load_values (void) ...@@ -291,19 +243,22 @@ load_values (void)
/** /**
* gedit_metadata_manager_get: * gedit_metadata_manager_get:
* @self: a #GeditMetadataManager.
* @location: a #GFile. * @location: a #GFile.
* @key: a key. * @key: a key.
* *
* Gets the value associated with the specified @key for the file @location. * Gets the value associated with the specified @key for the file @location.
*/ */
gchar * gchar *
gedit_metadata_manager_get (GFile *location, gedit_metadata_manager_get (GeditMetadataManager *self,
const gchar *key) GFile *location,
const gchar *key)
{ {
Item *item; Item *item;
gchar *value; gchar *value;
gchar *uri; gchar *uri;
g_return_val_if_fail (GEDIT_IS_METADATA_MANAGER (self), NULL);
g_return_val_if_fail (G_IS_FILE (location), NULL); g_return_val_if_fail (G_IS_FILE (location), NULL);
g_return_val_if_fail (key != NULL, NULL); g_return_val_if_fail (key != NULL, NULL);
...@@ -311,11 +266,11 @@ gedit_metadata_manager_get (GFile *location, ...@@ -311,11 +266,11 @@ gedit_metadata_manager_get (GFile *location,
gedit_debug_message (DEBUG_METADATA, "URI: %s --- key: %s", uri, key ); gedit_debug_message (DEBUG_METADATA, "URI: %s --- key: %s", uri, key );
if (!gedit_metadata_manager->values_loaded) if (!self->values_loaded)
{ {
gboolean res; gboolean res;
res = load_values (); res = gedit_metadata_manager_load_values (self);
if (!res) if (!res)
{ {
...@@ -324,8 +279,7 @@ gedit_metadata_manager_get (GFile *location, ...@@ -324,8 +279,7 @@ gedit_metadata_manager_get (GFile *location,
} }
} }
item = (Item *)g_hash_table_lookup (gedit_metadata_manager->items, item = (Item *)g_hash_table_lookup (self->items, uri);
uri);
g_free (uri); g_free (uri);
...@@ -347,6 +301,7 @@ gedit_metadata_manager_get (GFile *location, ...@@ -347,6 +301,7 @@ gedit_metadata_manager_get (GFile *location,
/** /**
* gedit_metadata_manager_set: * gedit_metadata_manager_set:
* @self: a #GeditMetadataManager.
* @location: a #GFile. * @location: a #GFile.
* @key: a key. * @key: a key.
* @value: the value associated with the @key. * @value: the value associated with the @key.
...@@ -354,13 +309,15 @@ gedit_metadata_manager_get (GFile *location, ...@@ -354,13 +309,15 @@ gedit_metadata_manager_get (GFile *location,
* Sets the @key to contain the given @value for the file @location. * Sets the @key to contain the given @value for the file @location.
*/ */
void void
gedit_metadata_manager_set (GFile *location, gedit_metadata_manager_set (GeditMetadataManager *self,
const gchar *key, GFile *location,
const gchar *value) const gchar *key,
const gchar *value)
{ {
Item *item; Item *item;
gchar *uri; gchar *uri;
g_return_if_fail (GEDIT_IS_METADATA_MANAGER (self));
g_return_if_fail (G_IS_FILE (location)); g_return_if_fail (G_IS_FILE (location));
g_return_if_fail (key != NULL); g_return_if_fail (key != NULL);
...@@ -368,11 +325,11 @@ gedit_metadata_manager_set (GFile *location, ...@@ -368,11 +325,11 @@ gedit_metadata_manager_set (GFile *location,
gedit_debug_message (DEBUG_METADATA, "URI: %s --- key: %s --- value: %s", uri, key, value); gedit_debug_message (DEBUG_METADATA, "URI: %s --- key: %s --- value: %s", uri, key, value);
if (!gedit_metadata_manager->values_loaded) if (!self->values_loaded)
{ {
gboolean ok; gboolean ok;
ok = load_values (); ok = gedit_metadata_manager_load_values (self);
if (!ok) if (!ok)
{ {
...@@ -381,14 +338,13 @@ gedit_metadata_manager_set (GFile *location, ...@@ -381,14 +338,13 @@ gedit_metadata_manager_set (GFile *location,
} }
} }
item = (Item *)g_hash_table_lookup (gedit_metadata_manager->items, item = (Item *)g_hash_table_lookup (self->items, uri);
uri);
if (item == NULL) if (item == NULL)
{ {
item = g_new0 (Item, 1); item = g_new0 (Item, 1);
g_hash_table_insert (gedit_metadata_manager->items, g_hash_table_insert (self->items,
g_strdup (uri), g_strdup (uri),
item); item);
} }
...@@ -417,7 +373,7 @@ gedit_metadata_manager_set (GFile *location, ...@@ -417,7 +373,7 @@ gedit_metadata_manager_set (GFile *location,
g_free (uri); g_free (uri);
gedit_metadata_manager_arm_timeout (); gedit_metadata_manager_arm_timeout (self);
} }
static void static void
...@@ -489,59 +445,60 @@ save_item (const gchar *key, const gpointer *data, xmlNodePtr parent) ...@@ -489,59 +445,60 @@ save_item (const gchar *key, const gpointer *data, xmlNodePtr parent)
xml_node); xml_node);
} }
static void static const gchar *
get_oldest (const gchar *key, const gpointer value, const gchar ** key_to_remove) gedit_metadata_manager_get_oldest (GeditMetadataManager *self)
{ {
const Item *item = (const Item *)value; GHashTableIter iter;
gpointer key, value, key_to_remove = NULL;
const Item *item_to_remove = NULL;
if (*key_to_remove == NULL) g_hash_table_iter_init (&iter, self->items);
{ while (g_hash_table_iter_next (&iter, &key, &value))
*key_to_remove = key;
}
else
{ {
const Item *item_to_remove = const Item *item = (const Item *) value;
g_hash_table_lookup (gedit_metadata_manager->items,
*key_to_remove);
g_return_if_fail (item_to_remove != NULL);
if (item->atime < item_to_remove->atime) if (key_to_remove == NULL)
{ {
*key_to_remove = key; key_to_remove = key;
item_to_remove = item;
}
else
{
g_return_val_if_fail (item_to_remove != NULL, NULL);
if (item->atime < item_to_remove->atime)
key_to_remove = key;
} }
} }
return key_to_remove;
} }
static void static void
resize_items (void) gedit_metadata_manager_resize_items (GeditMetadataManager *self)
{ {
while (g_hash_table_size (gedit_metadata_manager->items) > MAX_ITEMS) while (g_hash_table_size (self->items) > MAX_ITEMS)
{ {
gpointer key_to_remove = NULL; const gchar *key_to_remove;
g_hash_table_foreach (gedit_metadata_manager->items,
(GHFunc)get_oldest,
&key_to_remove);
key_to_remove = gedit_metadata_manager_get_oldest (self);
g_return_if_fail (key_to_remove != NULL); g_return_if_fail (key_to_remove != NULL);
g_hash_table_remove (self->items,
g_hash_table_remove (gedit_metadata_manager->items,
key_to_remove); key_to_remove);
} }
} }
static gboolean static gboolean
gedit_metadata_manager_save (gpointer data) gedit_metadata_manager_save (GeditMetadataManager *self)
{ {
xmlDocPtr doc; xmlDocPtr doc;
xmlNodePtr root; xmlNodePtr root;
gedit_debug (DEBUG_METADATA); gedit_debug (DEBUG_METADATA);
gedit_metadata_manager->timeout_id = 0; self->timeout_id = 0;
resize_items (); gedit_metadata_manager_resize_items (self);
xmlIndentTreeOutput = TRUE; xmlIndentTreeOutput = TRUE;
...@@ -553,22 +510,22 @@ gedit_metadata_manager_save (gpointer data) ...@@ -553,22 +510,22 @@ gedit_metadata_manager_save (gpointer data)
root = xmlNewDocNode (doc, NULL, (const xmlChar *)"metadata", NULL); root = xmlNewDocNode (doc, NULL, (const xmlChar *)"metadata", NULL);
xmlDocSetRootElement (doc, root); xmlDocSetRootElement (doc, root);
g_hash_table_foreach (gedit_metadata_manager->items,