- use _NM_GET_PRIVATE() and _NM_GET_PRIVATE_PTR() everywhere.
- reorder statements, to have GObject related functions (init, dispose,
constructed) at the bottom of each file and in a consistent order w.r.t.
each other.
- unify whitespaces in signal and properties declarations.
- use NM_GOBJECT_PROPERTIES_DEFINE() and _notify()
- drop unused signal slots in class structures
- drop unused header files for device factories
A D-Bus signal is asynchronous and it can happen that nm-dhcp-helper
emits the "Event" signal before the server is able to register a handler:
NM_DHCP_HELPER=/usr/libexec/nm-dhcp-helper
nmcli general logging level TRACE
for i in `seq 1 500`; do $NM_DHCP_HELPER & done
journalctl -u NetworkManager --since '1 min ago' | grep "didn't have associated interface" | wc -l
499
Avoid that, by calling the synchronous D-Bus method "Notify".
Interestingly, this race seem to exist since 2007.
Actually, we called g_dbus_connection_signal_subscribe() from inside
GDBusServer:new-connection signal. So it is not clear how such a race
could exist. I was not able to reproduce it by putting a sleep
before g_dbus_connection_signal_subscribe(). On the other hand, there
is bug rh#1372854 and above reproducer which strongly indicates that
events can be lost under certain circumstances.
Now we instead g_dbus_connection_register_object() from the
new-connection signal. According to my tests there was no more race
as also backed by glib's documentation. Still, keep a simple retry-loop
in nm-dhcp-helper just to be sure.
https://bugzilla.redhat.com/show_bug.cgi?id=1372854https://bugzilla.redhat.com/show_bug.cgi?id=1373276
Commit 4c7fa8dfdc ("core: drop root requirement for
load_connection(s)/set_logging D-Bus calls") removed the enforcing of
permission in the daemon for such methods since the D-Bus daemon
configuration already does that. That change also allows clients to
send a request and not wait for a response, since we don't have to
check the caller credentials in the daemon.
In the future we might switch to polkit for these methods, breaking
clients that don't wait for a reponse, so it seems better to prevent
from beginning such behavior.
Fixes: 4c7fa8dfdc
(cherry picked from commit dd27b79c4e)
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
We inconsistently use gulong,guint,int types to store signal handler
id, but the type returned by g_signal_connect() is a gulong.
This has no practical consequences because a int/guint is enough to
store the value, however it is better to use a consistent type, also
because nm_clear_g_signal_handler() accepts a pointer to the signal id
and thus it must be always called with the same pointer type.
NMExportedObject now derives from GDBusObjectSkeleton, which is what
GDBusObjectManagerServer wants. The main GDBusConnection and each
private server connection now gets a new GDBusObjectManagerServer,
and exported objects are registered with that instead of individually
exporting each GDBusInterfaceSkeleton.
Previously exported objects were not referenced by the BusManager,
but instead removed from the exports hash via weak references. The
GDBusObjectManagerServer instead references exported objects, which
can make them live much longer than they did before.
Co-Authored-By: Thomas Haller <thaller@redhat.com>
With ObjectManager we can not export ObjectSkeletons to multiple connections --
the manager would unexport the InterfaceSkeletons upon its destruction.
It seems easiest to just drop the private socket altogether; It was broken for
broken for some time and noone noticed anyway. Also startup before D-Bus is
still broken: NetworkManager would reconnect to the bus but multiple managers
won't notice the bus is around (we'll never see firewalld or policykit come up).
We should probably just stop pretending we support operation without a real
D-Bus server. With the advent of kdbus this makes even more sense.
Allows to enable more-asserts more granularly.
Unfortunately, the old check was "${enable_more_asserts} == "yes", thus
we cannot extend "--enable-more-assert=level" because that would mean
that the same build script cannot set the option on both old and new
NetworkManager.
Thus, add a new option --with-more-asserts=level. If you put the
following in your build script, it will work as expected whether
you build a new or an old version of NetworkManager.
./configure --enable-more-asserts --with-more-asserts=5
The 'new-connection' signal of a GDBusServer is emitted by default
through an idle source and the actual message processing starts only
after a signal handler returns TRUE.
Thus, before the signal handler has the chance to run, the GDBus
worker thread may detect that the connection is closed and schedule
the delivery of the 'closed' signal through another idle source.
After the termination of the 'new-connection' handler, the 'closed'
handler is executed, which cancels the subscription to GDBus signals
before any message has been processed.
This looks like a bug in GDBusServer; to work around it, just delay
the close of connection to let the signal dispatch run first.
https://bugzilla.gnome.org/show_bug.cgi?id=755170
Previously, nm_bus_manager_register_object() would take various D-Bus
skeleton objects that were associated with one NMExportedObject.
This was confusing, in that these skeleton objects are all for the
same NMObject but correspond to different D-Bus interfaces.
Also, setting the D-Bus property "Managed" of a Device is broken
because we might retrieve the wrong skeleton.
Now, the NMBusManager has a reference to the exported object directly.
The skeleton interface instances instead are now exposed by the NMExportedObject.
The signal "notify:g-name-owner" is only emitted for well-known
names, but not for unique connection names (":1.x") such as the secret
agent's connection. Also, it will not be emited for private connections.
That meant that when the client disconnected without explicitly
unregistering, the NMSecretAgent was not cleaned up and leaked
indefinitely.
As only one instance of secret agent with a certain 'identifier/uid'
pair can register, this bug also prevented the client from registering
until restart of NetworkManager.
Fixes: df6706813a
Previously, the order of destructing singleton instances
was undefined. Now, have singleton instances register their
destruction via nm_singleton_instance_register().
Objects that are registered later, will be destructed earlier. IOW,
they will be destroyed in reverse order of construction.
This is only a crude method to get the lifetime of singleton instances
right by default. Having singletons ref other singletons to keep them
alive gives more control over the lifetimes of singletons. This change
of having a defined order of destruction does not conflict with taking
references to singletons (and thus extending their lifetime).
Note that previously, NMPlatform was not registered for destruction.
We don't change that yet and intenionally leak a reference.
Our gdbus generated types use the same names as their corresponding
"real" types, but with "NM" changed to "NMDBus".
Unfortunately, that means that introspection/nmdbus-manager.c (the
generated type for src/nm-manager.c) uses the same type name as the
entirely unrelated src/nm-dbus-manager.c.
Fix this by removing the "d" from src/nm-dbus-manager.c. (We could
rename the generated type instead, but then it becomes inconsistent
with all the other generated types, and we're already using it as
"NMDBusManager" in libnm/nm-manager.c.)
