Re: [libvirt] Entering freeze for libvirt-4.6.0

On Tue, Jul 31, 2018 at 10:26:41AM +0200, Michal Privoznik wrote: > On 07/30/2018 05:20 PM, Andrea Bolognani wrote: > > On Sat, 2018-07-28 at 21:56 +0800, Daniel Veillard wrote: > > > > > > > > Unfortunately I've spotted an issue during my testing of rc1 today: > > with the libvirt_guest NSS module enabled, Evolution would crash a > > few seconds after being started. Here's the stack trace: > > > > #0 0x00007fffe7b69ba5 in json_object_iter_next () at /lib64/libjson-glib-1.0.so.0 > > #1 0x00007fffad8e757b in virJSONValueFromJansson () at /lib64/libnss_libvirt_guest.so.2 > > #2 0x00007fffad8e75d8 in virJSONValueFromJansson () at /lib64/libnss_libvirt_guest.so.2 > > #3 0x00007fffad8e8994 in virJSONValueFromString () at /lib64/libnss_libvirt_guest.so.2 > > #4 0x00007fffad8ecb5a in virMacMapNew () at /lib64/libnss_libvirt_guest.so.2 > > #5 0x00007fffad8cc140 in findLease () at /lib64/libnss_libvirt_guest.so.2 > > #6 0x00007fffad8ccb1c in _nss_libvirt_guest_gethostbyname4_r () at /lib64/libnss_libvirt_guest.so.2 > > #7 0x00007fffeb2599d2 in gaih_inet.constprop () at /lib64/libc.so.6 > > #8 0x00007fffeb25aab4 in getaddrinfo () at /lib64/libc.so.6 > > #9 0x00007ffff1d41a04 in do_lookup_by_name () at /lib64/libgio-2.0.so.0 > > #10 0x00007ffff1d3e937 in g_task_thread_pool_thread () at /lib64/libgio-2.0.so.0 > > #11 0x00007ffff5c39933 in g_thread_pool_thread_proxy () at /lib64/libglib-2.0.so.0 > > #12 0x00007ffff5c38f2a in g_thread_proxy () at /lib64/libglib-2.0.so.0 > > #13 0x00007ffff6314594 in start_thread () at /lib64/libpthread.so.0 > > #14 0x00007fffeb2700df in clone () at /lib64/libc.so.6 > > > > I've talked about it with a few colleagues and we believe the issue > > to be caused by jansson and json-glib both exporting a symbol called > > json_object_iter_next: Evolution itself (indirectly?) links against > > the latter library, so when the libvirt_guest NSS module is loaded > > and attempts to process JSON using the former, it picks up the wrong > > implementation, leading to a crash. gnome-boxes also crashes with > > the same stack trace. > > Worse. querying gentoo portage I've found some important packages > requiring json-glib: > > x11-libs/gtk > gnome-base/gnome-shell > > So once users of these app update to latest libvirt they will see the > crashes. > > > > > It seems like a similar issue could affect any application linking > > both to libvirt and json-glib, regardless of whether or not the NSS > > plugin has been enabled, which is of course pretty bad. > > Yes, any application can crash. > > > > > Unfortunately, I don't have any bright ideas on how to solve this, > > so anyone who might: please step forward! We're just a few days > > away from the next release, and if we can't figure out a way around > > this soon I'm afraid the only reasonable course of action would be > > to (temporarily) revert the switch from yajl to jansson. > > > > Well, what if we linked with jansson statically? I'm not sure if it is > possible (and have no idea how to achieve that), but what if our dynamic > libraries we produce already contained jansson and thus linker would not > even try to resolve json_* symbols. > It could "help" (quotes for all the disadvantages that approach has). Not because it would not try to resolve it, but because we would have the `json_` symbols as 'local' thanks to our src/libvirt.syms. If the lib was added to our dynamic lib we would still need to use `-Bsymbolic-functions` so that our `json_` symbols don't call `json_` symbols from the dynamic one programs where it is loaded. However that has some issues with `LD_PRELOAD`.

Maybe we could utilize the `-Bgroup` linker option, although I'm not sure how that is supposed to be used. In any case, this could be fixed in the respective libraries. The reasoning behind it is that since C doesn't support namespaces we namespace functions by a prefix (`vir` in libvirt), however that "namespace" needs to be unique. They should switch to `jansson_` or `glib_json_` prefixes and maybe provide macros for the previous names: #define json_auto_t jansson_auto_t ... I know it sounds like too big of a deal, but that's what happens in C world. The same would happen if libvirt used `json-glib` and some application linking with libvirt would start using jansson (and also use some specific functions). Not that we were guarded against that now. I'm not saying the release can go on, of course not, just that the ultimate fix is not something *we* should do.

Querying the fedora repositories I haven't found any similar situation. Projects that use jansson are separated from those that use json-c and those that use json-glib. If they want to use each other though, we'll be in the same mess as we are now.