GNOME Bugzilla – Bug 703192
The global log handler interferes with other application
Last modified: 2014-08-24 15:50:33 UTC
From IRC: <em> pecisk, I fear that https://git.gnome.org/browse/libgdata/commit/?id=59aea63 has some unintended consequencies: since it registers as a global log handles (not only for the gdat domain) it interacts badly with other users * em just spent a couple of hours tring to understand why I was not getting all the log messages I expected <em> :( <pecisk> em, it means after running tests it lefts this handle configured as in test code? <em> pecisk, I stepped totem in gdb and log messages from grilo were ending up in that handler <em> pecisk, it is set in gdata_service_init(), so I suppose it gets registered as soon as a GDataService gets instantiated, not only in test code
Created attachment 247974 [details] [review] Replacing g_log_set_default_handler with g_log_set_handler Replacing g_log_set_default_handler with g_log_set_handler which catches all levels of messages except for G_LOG_LEVEL_DEBUG, which is handled by tests/common.c when doing tests. This however means that G_LOG_LEVEL_DEBUG remains unhandled in case of normal use (not testing), and is printed out (if default handler remains unchanged). Not sure is this a problem though.
Comment on attachment 247974 [details] [review] Replacing g_log_set_default_handler with g_log_set_handler Works for me, thanks!
Review of attachment 247974 [details] [review]: Please commit this now to patch things up for everyone. It’s not a complete fix because it breaks the behaviour of the LIBGDATA_DEBUG environment variable, by not handling debug messages, so a further fix will be needed. What about moving the gdata_test_debug_output() and gdata_test_debug_handler() into libgdata itself (rather than in the test suite) so they can be integrated with the normal libgdata debug handler? The printerr handler would remain in the test suite. The code moved into libgdata would need adapting to limit its memory usage (e.g. by only keeping the most recent X messages), and would need checking to make sure it wasn’t storing sensitive data like passwords. What do you think, Pēteris?
commit 13e3571490d41aaff4761df12f0d97d5b048a5d1 Author: Peteris Krisjanis <pecisk@gmail.com> Date: Sat Jun 29 21:34:21 2013 +0300 Replacing g_log_set_default_handler with g_log_set_handler Replacing g_log_set_default_handler with g_log_set_handler for all message t Closes: https://bugzilla.gnome.org/show_bug.cgi?id=703192 gdata/gdata-service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Review of attachment 247974 [details] [review]: Patch comitted.
Philip: sounds worth investigating, moving code shouldn't be big problem. I will think how to limit amount of messages we store.
Created attachment 284348 [details] [review] build: Enable automake parallel-tests This should speed up the test runs somewhat, as well as improving log output handling.
Created attachment 284349 [details] [review] tests: Remove debug log storage system automake's parallel-tests feature now automatically separates and stores logs, and only prints them out on test failure. This is exactly what the logging code in the tests was doing before, so the logging code can now be simplified.
I decided it isn't worth putting the XML formatting code in libgdata itself, given how rarely the logs are used. The logging code in the tests can now be simplified though, due to the switch to automake's parallel-tests. Attachment 284348 [details] pushed as 8442be8 - build: Enable automake parallel-tests Attachment 284349 [details] pushed as c55465b - tests: Remove debug log storage system