codegen: Optimize server-side GDBus reply handling · frida/vala@74a66f9 · GitHub
Skip to content

Commit

Permalink
codegen: Optimize server-side GDBus reply handling
Browse files Browse the repository at this point in the history
- Don't send a reply if NO_REPLY_EXPECTED is set. This is already taken
  care of by g_dbus_method_invocation_return_gerror(), so we only need
  to do this for the non-error case, since we generate the reply message
  and manually send it.
- Because we have to keep arguments alive for the duration of the call,
  and this requires us to pass a ready callback even in case of
  NO_REPLY_EXPECTED, there is a challenge: If the implementation is a
  GDBus proxy, the presence of a ready callback means we don't set
  NO_REPLY_EXPECTED, even if we don't want the reply. But since we know
  that a proxy will copy the arguments right away, we can omit the ready
  callback in that case.
  • Loading branch information
oleavr committed Nov 6, 2021
1 parent 760ef64 commit 74a66f9
Showing 5 changed files with 273 additions and 10 deletions.
68 changes: 60 additions & 8 deletions codegen/valagdbusservermodule.vala
Original file line number Diff line number Diff line change
@@ -115,6 +115,31 @@ public class Vala.GDBusServerModule : GDBusClientModule {
ccode.add_assignment (ready_data_expr, ready_data_alloc);

ccode.add_assignment (new CCodeMemberAccess.pointer (ready_data_expr, "_invocation_"), new CCodeIdentifier ("invocation"));

ccode.add_declaration ("gboolean", new CCodeVariableDeclarator ("_fire_and_forget", new CCodeConstant ("FALSE")));
ccode.add_declaration ("GAsyncReadyCallback", new CCodeVariableDeclarator ("_callback_func",
new CCodeCastExpression (new CCodeIdentifier (wrapper_name + "_ready"), "GAsyncReadyCallback")));
ccode.add_declaration ("gpointer", new CCodeVariableDeclarator ("_callback_data", ready_data_expr));

var message_expr = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_method_invocation_get_message"));
message_expr.add_argument (new CCodeIdentifier ("invocation"));

var get_flags = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_get_flags"));
get_flags.add_argument (message_expr);
var no_reply_expected = new CCodeBinaryExpression (CCodeBinaryOperator.BITWISE_AND, get_flags, new CCodeConstant ("G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED"));

var is_proxy = new CCodeFunctionCall (new CCodeIdentifier ("G_IS_DBUS_PROXY"));
is_proxy.add_argument (new CCodeIdentifier ("self"));

var no_reply_and_arguments_copied = new CCodeBinaryExpression (CCodeBinaryOperator.AND, no_reply_expected, is_proxy);

ccode.open_if (no_reply_and_arguments_copied);

ccode.add_assignment (new CCodeIdentifier ("_fire_and_forget"), new CCodeConstant ("TRUE"));
ccode.add_assignment (new CCodeIdentifier ("_callback_func"), new CCodeConstant ("NULL"));
ccode.add_assignment (new CCodeIdentifier ("_callback_data"), new CCodeConstant ("NULL"));

ccode.close ();
}

foreach (Parameter param in m.get_parameters ()) {
@@ -186,7 +211,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
ccode.add_expression (free_error);

if (need_goto_label || requires_destroy (owned_type)) {
ccode.add_goto ("_error");
ccode.add_goto ("_return");
need_goto_label = true;
} else {
ccode.add_return ();
@@ -271,8 +296,8 @@ public class Vala.GDBusServerModule : GDBusClientModule {
}

if (m.coroutine && !ready) {
ccall.add_argument (new CCodeCastExpression (new CCodeIdentifier (wrapper_name + "_ready"), "GAsyncReadyCallback"));
ccall.add_argument (ready_data_expr);
ccall.add_argument (new CCodeIdentifier ("_callback_func"));
ccall.add_argument (new CCodeIdentifier ("_callback_data"));
}

if (!m.coroutine || ready) {
@@ -301,21 +326,40 @@ public class Vala.GDBusServerModule : GDBusClientModule {
ccode.add_expression (free_error);

if (need_goto_label) {
ccode.add_goto ("_error");
ccode.add_goto ("_return");
} else {
ccode.add_return ();
}

ccode.close ();
}

ccode.add_declaration ("GDBusMessage*", new CCodeVariableDeclarator ("_call_message"));
ccode.add_declaration ("GDBusMessage*", new CCodeVariableDeclarator.zero ("_reply_message", new CCodeConstant ("NULL")));

var message_expr = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_method_invocation_get_message"));
message_expr.add_argument (new CCodeIdentifier ("invocation"));
ccode.add_assignment (new CCodeIdentifier ("_call_message"), message_expr);

ccall = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_get_flags"));
ccall.add_argument (new CCodeIdentifier ("_call_message"));
var no_reply_expected = new CCodeBinaryExpression (CCodeBinaryOperator.BITWISE_AND, ccall, new CCodeConstant ("G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED"));
ccode.open_if (no_reply_expected);

var unref_call = new CCodeFunctionCall (new CCodeIdentifier ("g_object_unref"));
unref_call.add_argument (new CCodeIdentifier ("invocation"));
ccode.add_expression (unref_call);

if (need_goto_label) {
ccode.add_goto ("_return");
} else {
ccode.add_return ();
}

ccode.close ();

ccall = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_new_method_reply"));
ccall.add_argument (message_expr);
ccall.add_argument (new CCodeIdentifier ("_call_message"));
ccode.add_assignment (new CCodeIdentifier ("_reply_message"), ccall);

ccode.add_declaration ("GVariant*", new CCodeVariableDeclarator ("_reply"));
@@ -431,11 +475,15 @@ public class Vala.GDBusServerModule : GDBusClientModule {
}

if (need_goto_label) {
ccode.add_label ("_error");
ccode.add_label ("_return");
}

if (ready_data_expr != null && !ready) {
ccode.open_if (new CCodeIdentifier ("_fire_and_forget"));
}

foreach (Parameter param in m.get_parameters ()) {
if ((param.direction == ParameterDirection.IN && (ready_data_expr == null || ready)) ||
if ((param.direction == ParameterDirection.IN) ||
(param.direction == ParameterDirection.OUT && !no_reply && (!m.coroutine || ready))) {
if (param.variable_type is ObjectType && param.variable_type.type_symbol.get_full_name () == "GLib.Cancellable") {
continue;
@@ -472,7 +520,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
}
}

if (ready) {
if (ready_data_expr != null) {
var freecall = new CCodeFunctionCall (new CCodeIdentifier ("g_slice_free"));
freecall.add_argument (new CCodeIdentifier (ready_data_struct_name));
freecall.add_argument (ready_data_expr);
@@ -481,6 +529,10 @@ public class Vala.GDBusServerModule : GDBusClientModule {
ccode.add_statement (new CCodeEmptyStatement ());
}

if (ready_data_expr != null && !ready) {
ccode.close ();
}

pop_function ();

cfile.add_function_declaration (function);
4 changes: 3 additions & 1 deletion tests/Makefile.am
Original file line number Diff line number Diff line change
@@ -751,7 +751,9 @@ TESTS = \
dbus/async-bus.test \
dbus/async-connection.test \
dbus/async-errors.test \
dbus/async-no-reply.test \
dbus/async-no-reply-request.test \
dbus/async-no-reply-response.test \
dbus/async-no-reply-relay.test \
dbus/connection.test \
dbus/dbus-name-missing.test \
dbus/dynamic-method.test \
105 changes: 105 additions & 0 deletions tests/dbus/async-no-reply-relay.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
Packages: gio-2.0
D-Bus

Program: client

[DBus (name = "org.example.Test")]
interface Test : Object {
public abstract async string[] list_messages () throws IOError;
public abstract async void post_message (string message) throws IOError;
}

MainLoop main_loop;

async void run () {
Test test = yield Bus.get_proxy (BusType.SESSION, "org.example.Test", "/org/example/TestRelay");

string[] messages = yield test.list_messages ();
assert (messages.length == 0);

test.post_message.begin ("fire-and-forget");

messages = yield test.list_messages ();
assert (messages.length == 1);
assert (messages[0] == "fire-and-forget");

main_loop.quit ();
}

void main () {
run.begin ();

main_loop = new MainLoop (null, false);
main_loop.run ();
}

Program: server

[DBus (name = "org.example.Test")]
interface Test : Object {
public abstract async string[] list_messages () throws IOError;
public abstract async void post_message (string message) throws IOError;
}

class TestImpl : Object, Test {
private string[] messages = new string[0];

public async string[] list_messages () {
return messages;
}

public async void post_message (string message) {
messages += message;
}
}

MainLoop main_loop;

async void run () {
var conn = yield Bus.get (BusType.SESSION);

var events = new AsyncQueue<string> ();

conn.add_filter ((conn, message, incoming) => {
if (message.get_interface () == "org.example.Test" && message.get_member () != "ListMessages") {
switch (message.get_message_type ()) {
case DBusMessageType.METHOD_CALL:
events.push (message.get_flags ().to_string ());
break;
default:
assert_not_reached ();
}
}
return message;
});

conn.register_object ("/org/example/Test", new TestImpl () as Test);

var request_result = yield conn.call ("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName",
new Variant ("(su)", "org.example.Test", 0x4), null, 0, -1);
assert ((uint) request_result.get_child_value (0) == 1);

Test test = yield conn.get_proxy ("org.example.Test", "/org/example/Test");
conn.register_object ("/org/example/TestRelay", test);

Pid client_pid;
Process.spawn_async (null, { "dbus_async_no_reply_relay_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
ChildWatch.add (client_pid, (pid, status) => {
assert (status == 0);
run.callback ();
});
yield;

for (var i = 0; i < 3; i++)
assert (events.pop () == "G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED");
assert (events.try_pop () == null);

main_loop.quit ();
}

void main () {
run.begin ();

main_loop = new MainLoop (null, false);
main_loop.run ();
}
Original file line number Diff line number Diff line change
@@ -87,7 +87,7 @@ void main () {
assert ((uint) request_result.get_child_value (0) == 1);

Pid client_pid;
Process.spawn_async (null, { "dbus_async_no_reply_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
Process.spawn_async (null, { "dbus_async_no_reply_request_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
ChildWatch.add (client_pid, client_exit);

main_loop = new MainLoop ();
104 changes: 104 additions & 0 deletions tests/dbus/async-no-reply-response.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
Packages: gio-2.0
D-Bus

Program: client

[DBus (name = "org.example.Test")]
interface Test : Object {
public abstract async string[] list_messages () throws IOError;
public abstract async void post_message (string message) throws IOError;
}

MainLoop main_loop;

async void run () {
Test test = yield Bus.get_proxy (BusType.SESSION, "org.example.Test", "/org/example/Test");

var events = new AsyncQueue<string> ();
var calls = new HashTable<uint32, string> (null, null);

DBusConnection connection = ((DBusProxy) test).g_connection;
connection.add_filter ((conn, message, incoming) => {
if (message.get_interface () == "org.example.Test" && message.get_member () != "ListMessages") {
switch (message.get_message_type ()) {
case DBusMessageType.METHOD_CALL:
calls[message.get_serial ()] = message.get_member ();
break;
default:
assert_not_reached ();
}
}

if (incoming && message.get_message_type () == DBusMessageType.METHOD_RETURN) {
string? method_name = calls[message.get_reply_serial ()];
if (method_name != null)
events.push (method_name);
}

return message;
});

string[] messages = yield test.list_messages ();
assert (messages.length == 0);
assert (events.try_pop () == null);

yield test.post_message ("round-trip");
assert (events.pop () == "PostMessage");
assert (events.try_pop () == null);

test.post_message.begin ("fire-and-forget");

messages = yield test.list_messages ();
assert (messages.length == 2);
assert (messages[0] == "round-trip");
assert (messages[1] == "fire-and-forget");

assert (events.try_pop () == null);

main_loop.quit ();
}

void main () {
run.begin ();

main_loop = new MainLoop (null, false);
main_loop.run ();
}

Program: server

[DBus (name = "org.example.Test")]
class Test : Object {
private string[] messages = new string[0];

public async string[] list_messages () {
return messages;
}

public async void post_message (string message) {
messages += message;
}
}

MainLoop main_loop;

void client_exit (Pid pid, int status) {
assert (status == 0);
main_loop.quit ();
}

void main () {
var conn = Bus.get_sync (BusType.SESSION);
conn.register_object ("/org/example/Test", new Test ());

var request_result = conn.call_sync ("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName",
new Variant ("(su)", "org.example.Test", 0x4), null, 0, -1);
assert ((uint) request_result.get_child_value (0) == 1);

Pid client_pid;
Process.spawn_async (null, { "dbus_async_no_reply_response_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
ChildWatch.add (client_pid, client_exit);

main_loop = new MainLoop ();
main_loop.run ();
}

0 comments on commit 74a66f9

Please sign in to comment.