Skip to content

Commit 74a66f9

Browse files
oleavroleavr
committed
codegen: Optimize server-side GDBus reply handling
- 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.
1 parent 760ef64 commit 74a66f9

5 files changed

Lines changed: 273 additions & 10 deletions

File tree

‎codegen/valagdbusservermodule.vala‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,31 @@ public class Vala.GDBusServerModule : GDBusClientModule {
115115
ccode.add_assignment (ready_data_expr, ready_data_alloc);
116116

117117
ccode.add_assignment (new CCodeMemberAccess.pointer (ready_data_expr, "_invocation_"), new CCodeIdentifier ("invocation"));
118+
119+
ccode.add_declaration ("gboolean", new CCodeVariableDeclarator ("_fire_and_forget", new CCodeConstant ("FALSE")));
120+
ccode.add_declaration ("GAsyncReadyCallback", new CCodeVariableDeclarator ("_callback_func",
121+
new CCodeCastExpression (new CCodeIdentifier (wrapper_name + "_ready"), "GAsyncReadyCallback")));
122+
ccode.add_declaration ("gpointer", new CCodeVariableDeclarator ("_callback_data", ready_data_expr));
123+
124+
var message_expr = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_method_invocation_get_message"));
125+
message_expr.add_argument (new CCodeIdentifier ("invocation"));
126+
127+
var get_flags = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_get_flags"));
128+
get_flags.add_argument (message_expr);
129+
var no_reply_expected = new CCodeBinaryExpression (CCodeBinaryOperator.BITWISE_AND, get_flags, new CCodeConstant ("G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED"));
130+
131+
var is_proxy = new CCodeFunctionCall (new CCodeIdentifier ("G_IS_DBUS_PROXY"));
132+
is_proxy.add_argument (new CCodeIdentifier ("self"));
133+
134+
var no_reply_and_arguments_copied = new CCodeBinaryExpression (CCodeBinaryOperator.AND, no_reply_expected, is_proxy);
135+
136+
ccode.open_if (no_reply_and_arguments_copied);
137+
138+
ccode.add_assignment (new CCodeIdentifier ("_fire_and_forget"), new CCodeConstant ("TRUE"));
139+
ccode.add_assignment (new CCodeIdentifier ("_callback_func"), new CCodeConstant ("NULL"));
140+
ccode.add_assignment (new CCodeIdentifier ("_callback_data"), new CCodeConstant ("NULL"));
141+
142+
ccode.close ();
118143
}
119144

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

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

273298
if (m.coroutine && !ready) {
274-
ccall.add_argument (new CCodeCastExpression (new CCodeIdentifier (wrapper_name + "_ready"), "GAsyncReadyCallback"));
275-
ccall.add_argument (ready_data_expr);
299+
ccall.add_argument (new CCodeIdentifier ("_callback_func"));
300+
ccall.add_argument (new CCodeIdentifier ("_callback_data"));
276301
}
277302

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

303328
if (need_goto_label) {
304-
ccode.add_goto ("_error");
329+
ccode.add_goto ("_return");
305330
} else {
306331
ccode.add_return ();
307332
}
308333

309334
ccode.close ();
310335
}
311336

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

314340
var message_expr = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_method_invocation_get_message"));
315341
message_expr.add_argument (new CCodeIdentifier ("invocation"));
342+
ccode.add_assignment (new CCodeIdentifier ("_call_message"), message_expr);
343+
344+
ccall = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_get_flags"));
345+
ccall.add_argument (new CCodeIdentifier ("_call_message"));
346+
var no_reply_expected = new CCodeBinaryExpression (CCodeBinaryOperator.BITWISE_AND, ccall, new CCodeConstant ("G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED"));
347+
ccode.open_if (no_reply_expected);
348+
349+
var unref_call = new CCodeFunctionCall (new CCodeIdentifier ("g_object_unref"));
350+
unref_call.add_argument (new CCodeIdentifier ("invocation"));
351+
ccode.add_expression (unref_call);
352+
353+
if (need_goto_label) {
354+
ccode.add_goto ("_return");
355+
} else {
356+
ccode.add_return ();
357+
}
358+
359+
ccode.close ();
316360

317361
ccall = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_message_new_method_reply"));
318-
ccall.add_argument (message_expr);
362+
ccall.add_argument (new CCodeIdentifier ("_call_message"));
319363
ccode.add_assignment (new CCodeIdentifier ("_reply_message"), ccall);
320364

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

433477
if (need_goto_label) {
434-
ccode.add_label ("_error");
478+
ccode.add_label ("_return");
479+
}
480+
481+
if (ready_data_expr != null && !ready) {
482+
ccode.open_if (new CCodeIdentifier ("_fire_and_forget"));
435483
}
436484

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

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

532+
if (ready_data_expr != null && !ready) {
533+
ccode.close ();
534+
}
535+
484536
pop_function ();
485537

486538
cfile.add_function_declaration (function);

‎tests/Makefile.am‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,9 @@ TESTS = \
751751
dbus/async-bus.test \
752752
dbus/async-connection.test \
753753
dbus/async-errors.test \
754-
dbus/async-no-reply.test \
754+
dbus/async-no-reply-request.test \
755+
dbus/async-no-reply-response.test \
756+
dbus/async-no-reply-relay.test \
755757
dbus/connection.test \
756758
dbus/dbus-name-missing.test \
757759
dbus/dynamic-method.test \

‎tests/dbus/async-no-reply-relay.test‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
Packages: gio-2.0
2+
D-Bus
3+
4+
Program: client
5+
6+
[DBus (name = "org.example.Test")]
7+
interface Test : Object {
8+
public abstract async string[] list_messages () throws IOError;
9+
public abstract async void post_message (string message) throws IOError;
10+
}
11+
12+
MainLoop main_loop;
13+
14+
async void run () {
15+
Test test = yield Bus.get_proxy (BusType.SESSION, "org.example.Test", "/org/example/TestRelay");
16+
17+
string[] messages = yield test.list_messages ();
18+
assert (messages.length == 0);
19+
20+
test.post_message.begin ("fire-and-forget");
21+
22+
messages = yield test.list_messages ();
23+
assert (messages.length == 1);
24+
assert (messages[0] == "fire-and-forget");
25+
26+
main_loop.quit ();
27+
}
28+
29+
void main () {
30+
run.begin ();
31+
32+
main_loop = new MainLoop (null, false);
33+
main_loop.run ();
34+
}
35+
36+
Program: server
37+
38+
[DBus (name = "org.example.Test")]
39+
interface Test : Object {
40+
public abstract async string[] list_messages () throws IOError;
41+
public abstract async void post_message (string message) throws IOError;
42+
}
43+
44+
class TestImpl : Object, Test {
45+
private string[] messages = new string[0];
46+
47+
public async string[] list_messages () {
48+
return messages;
49+
}
50+
51+
public async void post_message (string message) {
52+
messages += message;
53+
}
54+
}
55+
56+
MainLoop main_loop;
57+
58+
async void run () {
59+
var conn = yield Bus.get (BusType.SESSION);
60+
61+
var events = new AsyncQueue<string> ();
62+
63+
conn.add_filter ((conn, message, incoming) => {
64+
if (message.get_interface () == "org.example.Test" && message.get_member () != "ListMessages") {
65+
switch (message.get_message_type ()) {
66+
case DBusMessageType.METHOD_CALL:
67+
events.push (message.get_flags ().to_string ());
68+
break;
69+
default:
70+
assert_not_reached ();
71+
}
72+
}
73+
return message;
74+
});
75+
76+
conn.register_object ("/org/example/Test", new TestImpl () as Test);
77+
78+
var request_result = yield conn.call ("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName",
79+
new Variant ("(su)", "org.example.Test", 0x4), null, 0, -1);
80+
assert ((uint) request_result.get_child_value (0) == 1);
81+
82+
Test test = yield conn.get_proxy ("org.example.Test", "/org/example/Test");
83+
conn.register_object ("/org/example/TestRelay", test);
84+
85+
Pid client_pid;
86+
Process.spawn_async (null, { "dbus_async_no_reply_relay_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
87+
ChildWatch.add (client_pid, (pid, status) => {
88+
assert (status == 0);
89+
run.callback ();
90+
});
91+
yield;
92+
93+
for (var i = 0; i < 3; i++)
94+
assert (events.pop () == "G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED");
95+
assert (events.try_pop () == null);
96+
97+
main_loop.quit ();
98+
}
99+
100+
void main () {
101+
run.begin ();
102+
103+
main_loop = new MainLoop (null, false);
104+
main_loop.run ();
105+
}

tests/dbus/async-no-reply.test renamed to tests/dbus/async-no-reply-request.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ void main () {
8787
assert ((uint) request_result.get_child_value (0) == 1);
8888

8989
Pid client_pid;
90-
Process.spawn_async (null, { "dbus_async_no_reply_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
90+
Process.spawn_async (null, { "dbus_async_no_reply_request_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
9191
ChildWatch.add (client_pid, client_exit);
9292

9393
main_loop = new MainLoop ();

‎tests/dbus/async-no-reply-response.test‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
Packages: gio-2.0
2+
D-Bus
3+
4+
Program: client
5+
6+
[DBus (name = "org.example.Test")]
7+
interface Test : Object {
8+
public abstract async string[] list_messages () throws IOError;
9+
public abstract async void post_message (string message) throws IOError;
10+
}
11+
12+
MainLoop main_loop;
13+
14+
async void run () {
15+
Test test = yield Bus.get_proxy (BusType.SESSION, "org.example.Test", "/org/example/Test");
16+
17+
var events = new AsyncQueue<string> ();
18+
var calls = new HashTable<uint32, string> (null, null);
19+
20+
DBusConnection connection = ((DBusProxy) test).g_connection;
21+
connection.add_filter ((conn, message, incoming) => {
22+
if (message.get_interface () == "org.example.Test" && message.get_member () != "ListMessages") {
23+
switch (message.get_message_type ()) {
24+
case DBusMessageType.METHOD_CALL:
25+
calls[message.get_serial ()] = message.get_member ();
26+
break;
27+
default:
28+
assert_not_reached ();
29+
}
30+
}
31+
32+
if (incoming && message.get_message_type () == DBusMessageType.METHOD_RETURN) {
33+
string? method_name = calls[message.get_reply_serial ()];
34+
if (method_name != null)
35+
events.push (method_name);
36+
}
37+
38+
return message;
39+
});
40+
41+
string[] messages = yield test.list_messages ();
42+
assert (messages.length == 0);
43+
assert (events.try_pop () == null);
44+
45+
yield test.post_message ("round-trip");
46+
assert (events.pop () == "PostMessage");
47+
assert (events.try_pop () == null);
48+
49+
test.post_message.begin ("fire-and-forget");
50+
51+
messages = yield test.list_messages ();
52+
assert (messages.length == 2);
53+
assert (messages[0] == "round-trip");
54+
assert (messages[1] == "fire-and-forget");
55+
56+
assert (events.try_pop () == null);
57+
58+
main_loop.quit ();
59+
}
60+
61+
void main () {
62+
run.begin ();
63+
64+
main_loop = new MainLoop (null, false);
65+
main_loop.run ();
66+
}
67+
68+
Program: server
69+
70+
[DBus (name = "org.example.Test")]
71+
class Test : Object {
72+
private string[] messages = new string[0];
73+
74+
public async string[] list_messages () {
75+
return messages;
76+
}
77+
78+
public async void post_message (string message) {
79+
messages += message;
80+
}
81+
}
82+
83+
MainLoop main_loop;
84+
85+
void client_exit (Pid pid, int status) {
86+
assert (status == 0);
87+
main_loop.quit ();
88+
}
89+
90+
void main () {
91+
var conn = Bus.get_sync (BusType.SESSION);
92+
conn.register_object ("/org/example/Test", new Test ());
93+
94+
var request_result = conn.call_sync ("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName",
95+
new Variant ("(su)", "org.example.Test", 0x4), null, 0, -1);
96+
assert ((uint) request_result.get_child_value (0) == 1);
97+
98+
Pid client_pid;
99+
Process.spawn_async (null, { "dbus_async_no_reply_response_client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
100+
ChildWatch.add (client_pid, client_exit);
101+
102+
main_loop = new MainLoop ();
103+
main_loop.run ();
104+
}

0 commit comments

Comments
 (0)