Created attachment 264301 [details] [review]
gdbus-codegen: Fix crasher in goa-using apps

When replacing a version of goa-daemon (from gnome-online-accounts)
by a newer version with some added properties, evolution-data-server
and the gvfs-goa volume monitor might crash as the extended interface
doesn't match the old one.

Work-around this by returning earlier from the _notify() implementation,
rather than accessing invalid memory.

The pocket patch is here:
https://bugzilla.gnome.org/show_bug.cgi?id=704564

1) Compile in jhbuild
2) jhbuild shell
3) ./goa-daemon --replace

gnome-control-center's online accounts panel, gvfs's owncloud support and e-d-s would crash when run with older (3.10) goa libraries against the newer daemon because of the added "ReadLater" property.

Review of attachment 264301 [details] [review]:

::: gio/gdbus-2.0/codegen/codegen.py
@@ +2771,3 @@
+                     '  _ExtendedGDBusInterfaceInfo *info = (_ExtendedGDBusInterfaceInfo *) g_dbus_interface_get_info (interface);\n'
+                     '  g_return_if_fail (info != NULL);\n'
+                     '  g_object_notify (G_OBJECT (object), info->hyphen_name);\n'

Hmm, we definitely want to support "old generated code talking to new daemon" ... because you might include the code in the binary - or statically link it.

This patch will cause spew to stderr which I think is wrong.

I actually thought we had test-cases for stuff like this? If not, someone should probably write those.

> because of the added "ReadLater" property.

FWIW, that analysis is wrong. The crash is caused because you've added a new _interface_, not a new property.

Created attachment 264396 [details] [review]
gdbus-codegen: Fix crasher in goa-using apps

When replacing a version of goa-daemon (from gnome-online-accounts)
by a newer version with some added interfaces, evolution-data-server
and the gvfs-goa volume monitor might crash as there's no interface
definition for this new interface.

Work-around this by returning earlier from the _notify() implementation,
rather than accessing invalid memory.

Review of attachment 264396 [details] [review]:

::: gio/gdbus-2.0/codegen/codegen.py
@@ +2773,3 @@
+                     '  if (info == NULL);\n'
+                     '    return;\n'
+                     '  g_object_notify (G_OBJECT (object), info->hyphen_name);\n'

Unreachable code and probably not what we want! I'd drop the return and use "info != NULL" instead.

Created attachment 264426 [details] [review]
gdbus-codegen: Fix crasher in goa-using apps

When replacing a version of goa-daemon (from gnome-online-accounts)
by a newer version with some added interfaces, evolution-data-server
and the gvfs-goa volume monitor might crash as there's no interface
definition for this new interface.

Work-around this by returning earlier from the _notify() implementation,
rather than accessing invalid memory.

Review of attachment 264426 [details] [review]:

Looks good with one minor nit. Just commit it.

::: gio/gdbus-2.0/codegen/codegen.py
@@ +2770,3 @@
                      '{\n'
+                     '  _ExtendedGDBusInterfaceInfo *info = (_ExtendedGDBusInterfaceInfo *) g_dbus_interface_get_info (interface);\n'
+                     '  /* Old generated code talking to new daemon */\n'

Minor nit: suggest to be a bit more precise here, e.g.

 /* info can be NULL if the other end is using a D-Bus interface we don't know
  * anything about, for example old generated code in this process talking to
  * newer generated code in the other process.
  */

Created attachment 264427 [details] [review]
gdbus-codegen: Fix crasher in goa-using apps

When replacing a version of goa-daemon (from gnome-online-accounts)
by a newer version with some added interfaces, evolution-data-server
and the gvfs-goa volume monitor might crash as there's no interface
definition for this new interface.

Work-around this by returning earlier from the _notify() implementation,
rather than accessing invalid memory.

Review of attachment 264427 [details] [review]:

LGTM, thanks!

Pushed to glib-2-36, glib-2-38 and master.

Attachment 264427 [details] pushed as c300079 - gdbus-codegen: Fix crasher in goa-using apps