Skip to content

[th/libnm_jansson-2]

Thomas Haller requested to merge th/libnm_jansson-2 into master

There are three populare JSON libraries (jansson, json-c, json-glib) which have a symbol conflict. Until recently, they also didn't implement symbol versioning (soon they possibly will). That means, an application must not load two of these libraries together or the application will crash.

NetworkManager uses libjansson for libnm (team validation), for team device plugin and ovs device plugin.

In particular, for libnm this is a problem, because we don't want to prevent users of libnm to link against another JSON library. For the device plugins, it's less a problem, because we are more in control about what gets linked into NetworkManager binary.

libnm works around that by not directly linking to libjansson, but dlopening it with RTLD_LOCAL|RTLD_DEEPBIND.

Note that recently libcryptsetup started linking against json-c. This library is required by libmount, which in turn is required by glib, so it ends up in all our glib applications. That causes bad crashes, and must be fixed. We probably won't add the dlopen workaround here, because it's much effort and should be fixed by using symbol versioning (or not use such irresponsible libraries that don't have regard for symbol clashes). This libcryptsetup issue makes it especially bad ( https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1752626.html ) and here we have the conflict because between jansson and json-c.

Note that the issue in general exists for a long time, and for libnm the more common problem was that another application might drag in json-glib.

For libnm, we currently do a pretty ugly and confusing hack. We re-define the json symbols before including <jansson.h>. That is confusing. But it also doesn't seem to really work, because our libnm based unit test libnm-core/tests/test-general fails on Debian sid. I don't understand why it fails there, the dlopen workaround should function properly. But it's also really confusing to include <jansson.h> and trying to avoid using any symbols from it.


Rework how we do that. No longer include <jansson.h> in libnm. Instead, introduce NetworkManager specific wrappers that delegate to libjansson (loaded by dlopon). The difference is that IMO this approach is clearer. You can actually understand which code gets called and it doesn't depend on (conditionally) redefining names.

This resurrects an earlier merge commit https://github.com/NetworkManager/NetworkManager/pull/56 that got NACK'ed. I propose it here again, I think this approach is preferable.

Also, it does fix the unit test failures on Debian:sid for libnm. Of course, on the affected systems where libcryptsetup drags in libjson-c, the team and ovs plugins don't work (since they link to jansson directly). So, this does not fix #453 (closed) .

Merge request reports