Conversation
Fix modernize warnings in find_if calls. Fix unused include warning. Fix passing const values by value. Fix marking nodiscard warning Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Remove unused CPackIFW and CTest include statements, this solves amarula#18. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the D-Bus proxy and agent implementation to ensure thread safety by integrating GMainContext and g_main_context_invoke_full. It introduces a connectSignal helper and updates DBusProxy to execute D-Bus operations within the dedicated context. The PR also adopts C++20 ranges in examples and introduces a QtThreadBundle utility for multi-threaded testing. Review feedback identifies a syntax error in a static_cast, suggests popping thread-default contexts for proper cleanup, and warns that specific D-Bus proxy flags may prevent necessary property and signal updates.
| G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | | ||
| G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | |
There was a problem hiding this comment.
Using G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES and G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS prevents the proxy from loading initial properties and subscribing to D-Bus signals. However, the class relies on the g-signal::PropertyChanged signal (which maps to D-Bus signals) to update its internal state. With these flags set, the proxy will not receive any updates, and the props_ member will remain uninitialized or empty. If the goal was to avoid blocking during construction, consider that g_dbus_proxy_new_sync is already a blocking call.
|
🧩 Build Artifacts ✅ The following build artifacts were produced: |
Calling this method(remove) on Ethernet devices, hidden WiFi services or provisioned services will cause an error message. It is not possible to remove these kind of services. https://git.kernel.org/pub/scm/network/connman/connman.git/tree/doc/ service-api.txt#n98 Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Iterate all the test files in the CMake configuration in order to remove duplicated code. Define a class that iterates the global default context simulating what Qt libraries does. The latter class saves the thread id where the object is created and the thread id where the global default context it is iterated. Use the class on the tests to check that callbacks are not run on the main thread nor in the thread iterating the global default context. This helps to test the issue presented in amarula#21. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Create a thread default context and iterate the later on the worker thread. This solves amarula#21. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Wrap the callMethod method inside a g_main_context_invoke. The latter makes that the callbacks are executed in the worker thread provided by the library. This solves amarula#21. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Initialize the glib proxy member in the worker thread and connect the signals in the worker thread. All this make the callbacks of the connected signals to run on the worker thread. The creation of the proxy is blocked until the worker thread creates it. This solves amarula#21. Add flags when proxy is created to improve creation time. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
This solves amarula#21. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
Call the register object in the dbus in the worker thread. The latter makes the callback to be executed also in the worker thread. The creation of the agent block the thread until the worker thread register the object. This solves amarula#21. Signed-off-by: Eduardo Gonzalez <eduardo.gonzalez@amarulasolutions.com>
EddyTheCo
force-pushed
the
callbacks_in_worker_thread_move_proxy
branch
from
fafcf84 to
677d7a1
Compare
|
🧩 Build Artifacts ✅ The following build artifacts were produced: |
1 participant
The PR should address #21 and #18.
Still, there is some memory leak related to https://gitlab.gnome.org/GNOME/glib/-/issues/3926.