! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
weather plugin crashes when opening the forecast window
Status:
CLOSED: FIXED
Product:
Xfce4-weather-plugin
Component:
General

Comments

Description Guido Berhoerster 2012-08-06 15:06:11 CEST
Created attachment 4578 
backtrace

The weather plugin crashes when opening the forecast window, see attached backtrace.
Comment 1 Harald Judt 2012-08-06 15:40:58 CEST
My guess is it crashes because there is no data available (conditions->point == NULL). Only a symptom, not the real problem, but I should catch that error too so that it doesn't crash.

I didn't get around to adding debugging messages for 0.8.1, so information is a bit sparse. Since all works fine here, it's hard to tell what exactly is going wrong. Does the compiler print any warnings? Did 0.8.0 work for you? If so, could you bisect?

You might also try to replace the my_timegm function with the one here:
http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=c5e86912a6661c6a7b04d66c786693a01a2d45ea

Maybe that works better and gives a bit more info.
Comment 2 Guido Berhoerster 2012-08-06 18:57:12 CEST
Created attachment 4579 
build log

(In reply to comment #1)
> My guess is it crashes because there is no data available (conditions->point
> == NULL). Only a symptom, not the real problem, but I should catch that
> error too so that it doesn't crash.
> 
> I didn't get around to adding debugging messages for 0.8.1, so information
> is a bit sparse. Since all works fine here, it's hard to tell what exactly
> is going wrong. Does the compiler print any warnings? Did 0.8.0 work for
> you? If so, could you bisect?
> 
> You might also try to replace the my_timegm function with the one here:
> http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/
> ?id=c5e86912a6661c6a7b04d66c786693a01a2d45ea
> 
> Maybe that works better and gives a bit more info.

I've tried 0.8.0 and it segfaults as well. There are a couple of warnings but I don't see anything immediately obvious related to the crash in the build log, there are runtime warnings from glib assertion failures though. Note also (as I mentioned on the ML) that I don't get any forecasts for my location. Build log is attached.
Comment 3 Guido Berhoerster 2012-08-06 18:59:20 CEST
Here's the runtime warnings from 0.8.0:

(wrapper:28079): weather-WARNING **: getting http://api.yr.no//weatherapi/sunrise/1.0/?lat=53.5437641;lon=10.0099133;date=2012-07-28

(wrapper:28079): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=53.5437641;lon=10.0099133
**
weather:ERROR:weather-parsers.c:473:xml_time_free: assertion failed: (timeslice != NULL)

(xfce4-weather-plugin:28304): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=52.033298;lon=8.533300

(xfce4-weather-plugin:28304): Gdk-CRITICAL **: IA__gdk_window_get_origin: assertion `GDK_IS_WINDOW (window)' failed
xfce4-panel-Message: Plugin weather-14 has been automatically restarted after crash.

(xfce4-weather-plugin:28307): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=52.033298;lon=8.533300

(xfce4-weather-plugin:28307): Gdk-CRITICAL **: IA__gdk_window_get_origin: assertion `GDK_IS_WINDOW (window)' failed

(xfce4-weather-plugin:28310): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=52.033298;lon=8.533300




And here from 0.8.1:

(wrapper:28494): weather-WARNING **: getting http://api.yr.no//weatherapi/sunrise/1.0/?lat=52.033298;lon=8.533300;date=2012-07-28

(wrapper:28494): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=52.033298;lon=8.533300

(wrapper:28494): Gdk-CRITICAL **: IA__gdk_window_get_origin: assertion `GDK_IS_WINDOW (window)' failed
xfce4-panel-Message: Plugin weather-15 has been automatically restarted after crash.

(wrapper:28497): weather-WARNING **: getting http://api.yr.no//weatherapi/sunrise/1.0/?lat=52.033298;lon=8.533300;date=2012-07-28

(wrapper:28497): weather-WARNING **: getting http://api.yr.no//weatherapi/locationforecastlts/1.1/?lat=52.033298;lon=8.533300
Comment 4 Harald Judt 2012-08-06 19:43:32 CEST
[    9s] weather-parsers.c:57:5: warning: implicit declaration of function 'setenv' is invalid in C99 [-Wimplicit-function-declaration]
[    9s]     setenv("TZ", "", 1);
[    9s]     ^
[    9s] weather-parsers.c:63:9: warning: implicit declaration of function 'unsetenv' is invalid in C99 [-Wimplicit-function-declaration]
[    9s]         unsetenv("TZ");

I'm not sure, but I somehow feel like your problem is related to the my_timegm function. timegm is deprecated, my_timegm is the alternative provided by the related linux man page. In theory it should be portable, but on the net I found different solutions for different systems. Maybe there's some other #include or #define missing. You might try replacing line 115 in weather-parsers.c (function parse_xml_timestring) as follows if your platform supports timegm:

t = my_timegm(&tm);

with

t = timegm(&tm);

Does this make the crashes go away?

Now to the rest of the build log: I fixed a few warnings, but found nothing obvious that would explain your problem.

And as for the runtime output:
The GDK assert is a bit annoying but doesn't seem to hurt. It's getting printed everytime one opens the summary window and has to do with how the window size is calculated. The download "warnings" are in fact not warnings at all, but indicate that the plugin is fetching data.
Comment 5 Guido Berhoerster 2012-08-06 21:59:33 CEST
(In reply to comment #4)
> [    9s] weather-parsers.c:57:5: warning: implicit declaration of function
> 'setenv' is invalid in C99 [-Wimplicit-function-declaration]
> [    9s]     setenv("TZ", "", 1);
> [    9s]     ^
> [    9s] weather-parsers.c:63:9: warning: implicit declaration of function
> 'unsetenv' is invalid in C99 [-Wimplicit-function-declaration]
> [    9s]         unsetenv("TZ");

On Linux/glibc setenv/unsetenv requires _XOPEN_SOURCE >= 600 to be defined, so you need to replace the
#define _XOPEN_SOURCE
#define _XOPEN_SOURCE_EXTENDED 1
with
#define _XOPEN_SOURCE 600
on this platform, not sure about FreeBSD and others.

> I'm not sure, but I somehow feel like your problem is related to the
> my_timegm function. timegm is deprecated, my_timegm is the alternative
> provided by the related linux man page. In theory it should be portable, but
> on the net I found different solutions for different systems. Maybe there's
> some other #include or #define missing. You might try replacing line 115 in
> weather-parsers.c (function parse_xml_timestring) as follows if your
> platform supports timegm:
> 
> t = my_timegm(&tm);
> 
> with
> 
> t = timegm(&tm);
> 
> Does this make the crashes go away?

No, still the same crash.

> Now to the rest of the build log: I fixed a few warnings, but found nothing
> obvious that would explain your problem.
> 
> And as for the runtime output:
> The GDK assert is a bit annoying but doesn't seem to hurt. It's getting
> printed everytime one opens the summary window and has to do with how the
> window size is calculated. The download "warnings" are in fact not warnings
> at all, but indicate that the plugin is fetching data.

Right, you should probably use g_debug instead of g_warning (which will be hidden by default on glib >= 2.32 unless G_DEBUG is set). The GDK_IS_WINDOW (window) assertion failure however means that window is some other object/NULL/dangling pointer so it can be a hint something isn't right.
I'm a bit short of time currently, sorry that I can't look further into this right now.
Comment 6 Harald Judt 2012-08-06 22:39:51 CEST
(In reply to comment #5)
> (In reply to comment #4)
> > [    9s] weather-parsers.c:57:5: warning: implicit declaration of function
> > 'setenv' is invalid in C99 [-Wimplicit-function-declaration]
> > [    9s]     setenv("TZ", "", 1);
> > [    9s]     ^
> > [    9s] weather-parsers.c:63:9: warning: implicit declaration of function
> > 'unsetenv' is invalid in C99 [-Wimplicit-function-declaration]
> > [    9s]         unsetenv("TZ");
> 
> On Linux/glibc setenv/unsetenv requires _XOPEN_SOURCE >= 600 to be defined,
> so you need to replace the
> #define _XOPEN_SOURCE
> #define _XOPEN_SOURCE_EXTENDED 1
> with
> #define _XOPEN_SOURCE 600
> on this platform, not sure about FreeBSD and others.

Linux strptime needs #define _XOPEN_SOURCE, OpenBSD needs #define _XOPEN_SOURCE_EXTENDED 1: http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=94c7c6e88ed7caf1ed8fec608558050274064a84

So I hope setting _XOPEN_SOURCE 600 doesn't break it.

> > I'm not sure, but I somehow feel like your problem is related to the
> > my_timegm function. timegm is deprecated, my_timegm is the alternative
> > provided by the related linux man page. In theory it should be portable, but
> > on the net I found different solutions for different systems. Maybe there's
> > some other #include or #define missing. You might try replacing line 115 in
> > weather-parsers.c (function parse_xml_timestring) as follows if your
> > platform supports timegm:
> > 
> > t = my_timegm(&tm);
> > 
> > with
> > 
> > t = timegm(&tm);
> > 
> > Does this make the crashes go away?
> 
> No, still the same crash.

Then maybe we can exclude this as the cause.

> > And as for the runtime output:
> > The GDK assert is a bit annoying but doesn't seem to hurt. It's getting
> > printed everytime one opens the summary window and has to do with how the
> > window size is calculated. The download "warnings" are in fact not warnings
> > at all, but indicate that the plugin is fetching data.
> 
> Right, you should probably use g_debug instead of g_warning (which will be
> hidden by default on glib >= 2.32 unless G_DEBUG is set).

I will look into how other plugins do that, but yes that's what I had plans for.

> The GDK_IS_WINDOW
> (window) assertion failure however means that window is some other
> object/NULL/dangling pointer so it can be a hint something isn't right.

It appears to happen in some GDK library, maybe the widget is supposed to have a window but has not been realized by that time. It may not be vital for correct functioning though, otherwise more users would complain. Searching the web told me such messages are harmless. Don't know. But the crash you see is definitely caused by conditions being NULL. I will fix this, though it won't help much with your problem.

> I'm a bit short of time currently, sorry that I can't look further into this
> right now.

Since I'm unable to reproduce it here, it's probably easier to wait until I have debugging output implemented.
Comment 7 Guido Berhoerster 2012-08-07 01:23:08 CEST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > [    9s] weather-parsers.c:57:5: warning: implicit declaration of function
> > > 'setenv' is invalid in C99 [-Wimplicit-function-declaration]
> > > [    9s]     setenv("TZ", "", 1);
> > > [    9s]     ^
> > > [    9s] weather-parsers.c:63:9: warning: implicit declaration of function
> > > 'unsetenv' is invalid in C99 [-Wimplicit-function-declaration]
> > > [    9s]         unsetenv("TZ");
> > 
> > On Linux/glibc setenv/unsetenv requires _XOPEN_SOURCE >= 600 to be defined,
> > so you need to replace the
> > #define _XOPEN_SOURCE
> > #define _XOPEN_SOURCE_EXTENDED 1
> > with
> > #define _XOPEN_SOURCE 600
> > on this platform, not sure about FreeBSD and others.
> 
> Linux strptime needs #define _XOPEN_SOURCE, OpenBSD needs #define
> _XOPEN_SOURCE_EXTENDED 1:
> http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/
> ?id=94c7c6e88ed7caf1ed8fec608558050274064a84
> 
> So I hope setting _XOPEN_SOURCE 600 doesn't break it.

As I said, only on the Linux/glibc platform so it should probably be wrapped in a #ifdef __GLIBC__ (_XOPEN_SOURCE == 600 implies _XOPEN_SOURCE_EXTENDED == 1), on other platforms this usually triggers strict SUSv3 conformance which might not be desirable so for others I'd keep the _XOPEN_SOURCE and _XOPEN_SOURCE_EXTENDED 1.

> > Right, you should probably use g_debug instead of g_warning (which will be
> > hidden by default on glib >= 2.32 unless G_DEBUG is set).
> 
> I will look into how other plugins do that, but yes that's what I had plans
> for.

See e.g. https://bitbucket.org/gber/xfce4-mixer-improvements/src/e69a343e19f3/add-runtime-debugging.patch#cl-138 on how to do that.

> > The GDK_IS_WINDOW
> > (window) assertion failure however means that window is some other
> > object/NULL/dangling pointer so it can be a hint something isn't right.
> 
> It appears to happen in some GDK library, maybe the widget is supposed to
> have a window but has not been realized by that time. It may not be vital
> for correct functioning though, otherwise more users would complain.
> Searching the web told me such messages are harmless. Don't know. But the
> crash you see is definitely caused by conditions being NULL. I will fix
> this, though it won't help much with your problem.

Not necessarily harmless but not the issue here.
Comment 8 Guido Berhoerster 2012-08-07 17:18:51 CEST
Commit 	51d98a68ee19e6f2cfa044727395bf1fcb9e4b32 fixes the crash, I now also get the actual weather data shown instead of the "Short-term forecast data unavailable." error, so feel free to close this.
Comment 9 Harald Judt 2012-08-07 19:23:49 CEST
I'm glad that it works for you now. I wonder though why it does, and I'm not sure the fix in the commit you mentioned could actually have anything to do with it. It fixes the crash, yes, but the crash only happened when there was no data for "current conditions".

I'll leave it open until I have investigated and fixed the _XOPEN_SOURCE == 600 stuff.

Thanks for your information about debugging, it will be of great help.
Comment 10 Harald Judt 2012-08-15 10:45:56 CEST
For some reason I don't know, wrapping those declarations with #ifdef __GLIBC__ doesn't work on my system.

However, the man page states:

setenv(), unsetenv():
    _BSD_SOURCE || _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600

So I could also use _BSD_SOURCE to make it compile with -std=c99, and leave _XOPEN_SOURCE and _XOPEN_SOURCE_EXTENDED 1 where they are now. That seems to work and doesn't cause any errors for me, but I wonder whether it might be harmful elsewhere?

Those #defines are needed in weather-http.c for compilation with -stdc=c99 too.


--
diff --git a/panel-plugin/weather-http.c b/panel-plugin/weather-http.c
index 4f00e77..8ce1846 100644
--- a/panel-plugin/weather-http.c
+++ b/panel-plugin/weather-http.c
@@ -20,6 +20,12 @@
 #include <config.h>
 #endif
 
+/* Some library functions require the following defines to appear before the
+ * appropriate #includes, or compilation might fail. */
+#define _XOPEN_SOURCE
+#define _XOPEN_SOURCE_EXTENDED 1
+#define _BSD_SOURCE
+
 #include <sys/select.h>
 #include <sys/time.h>
 #include <sys/types.h>
diff --git a/panel-plugin/weather-parsers.c b/panel-plugin/weather-parsers.c
index c4afe39..d800c01 100644
--- a/panel-plugin/weather-parsers.c
+++ b/panel-plugin/weather-parsers.c
@@ -20,12 +20,14 @@
 #endif
 
 /*
- * The following two defines fix compile warnings and need to be
+ * The following defines fix compile warnings and need to appear
  * before time.h and libxfce4panel.h (which includes glib.h).
  * Otherwise, they will be ignored.
  */
 #define _XOPEN_SOURCE
 #define _XOPEN_SOURCE_EXTENDED 1
+#define _BSD_SOURCE
+
 #include "weather-parsers.h"
 #include <libxfce4panel/libxfce4panel.h>
 #include <time.h>
Comment 11 Guido Berhoerster 2012-08-15 11:17:21 CEST
Created attachment 4593 
use glib setenv/getenv/unsetenv wrappers

(In reply to comment #10)
I see that glib has wrappers for setenv/getenv/unsetenv which seem to be an even better and more portable solution. See attached patch.
Comment 12 Harald Judt 2012-08-15 11:50:49 CEST
Ok, I'll try your patch too, it seems to be a good solution, though the modifications in weather-http as pointed out previously will still be needed.
Comment 13 Harald Judt 2012-08-15 12:12:13 CEST
Works fine. I've fixed a const compile warning and pushed it to git:
http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=1b5db7a01a1be02ef088ac2defb76fd962c3267b
Comment 14 Harald Judt 2012-08-15 12:18:05 CEST
And I've pushed the fix for weather-http too, I hope it doesn't break anything:
http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=9c64229317f83c67186a65cc0c271cc5d28cebac

Since the initial problems have been fixed, I set this bug "resolved fixed" now and work on the debug code later. Feel free to reopen if any problems arise.
Comment 15 Harald Judt 2012-08-19 11:12:31 CEST
I've finally got around to implementing debugging code and pushed it to git, so solving problems should be easier in the future. The plugin dumps data at selected places that I found crucial. Thanks for your code example, I modified it a bit and hope I got it right: http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=7265a1596bd324e1e8d6fe66c9a95e0dedc3b170

Bug #9182

Reported by:
Guido Berhoerster
Reported on: 2012-08-06
Last modified on: 2012-09-19

People

Assignee:
Harald Judt
CC List:
2 users

Version

Attachments

backtrace (12.75 KB, text/plain)
2012-08-06 15:06 CEST , Guido Berhoerster
no flags
build log (108.62 KB, text/plain)
2012-08-06 18:57 CEST , Guido Berhoerster
no flags
use glib setenv/getenv/unsetenv wrappers (653 bytes, patch)
2012-08-15 11:17 CEST , Guido Berhoerster
no flags

Additional information