Skip to content

Commit

Permalink
dbus: add an external param waitOnDelay to handle service unavailable
Browse files Browse the repository at this point in the history
Sometimes the D-Bus service is unavailable at bootstraping the os, so
the client will get a connection with NULL, and the following operations
with this connection would make failures even abortions.

This patch intros a synchronous wait, that the user could gives a max
timeout, within that, ShadowNode attempts to connect and check the following
errors:

- org.freedesktop.DBus.Error.FileNotFound
- org.freedesktop.DBus.Error.Failed

If one of them, client would sleep 100ms and retry until the current time
is over the given max timeout. Otherwise it breaks the loop for other
errors like "Address does not contain a colon".
  • Loading branch information
yorkie committed Nov 10, 2018
1 parent 8b33127 commit 259e15a
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 15 deletions.
12 changes: 7 additions & 5 deletions src/js/dbus.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ function convertData2Array(data) {

/**
* @class Bus
* @param {String} name - the bus name
* @param {String} name - the bus name.
* @param {Number} delayOnWait - the total delay for waiting dbus connection.
*/
function Bus(name) {
function Bus(name, delayOnWait) {
EventEmitter.call(this);
this.name = name || 'session';
this.dbus = new DBus();
this.dbus.getBus(DBUS_TYPES[this.name]);
this.dbus.getBus(DBUS_TYPES[this.name], delayOnWait);
this.dbus.setSignalHandler(this.handleSignal.bind(this));
this._object = null;
}
Expand Down Expand Up @@ -512,13 +513,14 @@ ServiceInterface.prototype.update = util.deprecate(function() {
* @module dbus
* @method getBus
* @param {String} name
* @param {Number} delayOnWait
*/
function getBus(name) {
function getBus(name, delayOnWait) {
if (_busInstance === false) {
throw new Error('dbus connection has been destroyed');
}
if (_busInstance === null) {
_busInstance = new Bus(name);
_busInstance = new Bus(name, delayOnWait);
}
return _busInstance;
}
Expand Down
45 changes: 35 additions & 10 deletions src/modules/iotjs_module_dbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "iotjs_objectwrap.h"
#include <dbus/dbus.h>
#include <stdlib.h>
#include <unistd.h>

typedef struct {
iotjs_jobjectwrap_t jobjectwrap;
Expand All @@ -23,9 +24,10 @@ static iotjs_dbus_t* iotjs_dbus_create(const jerry_value_t jdbus);
static void iotjs_dbus_destroy(iotjs_dbus_t* dbus);
static void iotjs_dbus_watcher_close(void* data);

#define IOTJS_DBUS_CONN_DELAY 100 * 1000
#define IOTJS_DBUS_RELEASE_HANDLE() \
do { \
if (!_this->destroyed) { \
if (_this->connection != NULL && !_this->destroyed) { \
dbus_connection_unref(_this->connection); \
iotjs_dbus_watcher_close((void*)&_this->watcher); \
uv_close((uv_handle_t*)&_this->connection_handle, NULL); \
Expand Down Expand Up @@ -412,9 +414,6 @@ JS_FUNCTION(DbusConstructor) {
IOTJS_VALIDATED_STRUCT_METHOD(iotjs_dbus_t, dbus);

_this->connection_handle.data = _this;
uv_async_init(uv_default_loop(), &_this->connection_handle,
iotjs_dbus_connection_cb);

return jerry_create_undefined();
}

Expand All @@ -423,22 +422,48 @@ JS_FUNCTION(GetBus) {
IOTJS_VALIDATED_STRUCT_METHOD(iotjs_dbus_t, dbus);

DBusError error;
dbus_error_init(&error);

DJS_CHECK_ARGS(1, number);
int type = JS_GET_ARG(0, number);
if (type == 0 /* BUS_SYSTEM */) {
_this->connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);
} else if (type == 1 /* BUS_SESSION */) {
_this->connection = dbus_bus_get(DBUS_BUS_SESSION, &error);
int wait = 1000; // default is 1s
if (jerry_value_is_number(jargv[1])) {
wait = iotjs_jval_as_number(jargv[1]);
}
wait *= 1000;

// FIXME(Yorkie): currently the implementation waits synchronously because it
// might be used in edge-case, when the dbus-daemon is unavailable or the unix
// path is not found.
do {
dbus_error_init(&error);
if (type == 0 /* BUS_SYSTEM */) {
_this->connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);
} else if (type == 1 /* BUS_SESSION */) {
_this->connection = dbus_bus_get(DBUS_BUS_SESSION, &error);
}

if (_this->connection != NULL)
break;

fprintf(stderr, "DBUS(%s) %s\n", error.name, error.message);
if (wait <= 0 ||
(strcmp(error.name, "org.freedesktop.DBus.Error.FileNotFound") != 0 &&
strcmp(error.name, "org.freedesktop.DBus.Error.Failed") != 0)) {
break;
} else {
usleep(IOTJS_DBUS_CONN_DELAY);
wait -= IOTJS_DBUS_CONN_DELAY;
}
} while (_this->connection == NULL);

if (_this->connection == NULL) {
if (dbus_error_is_set(&error)) {
return JS_CREATE_ERROR(COMMON, error.message);
} else {
return JS_CREATE_ERROR(COMMON, "failed to get dbus object");
}
} else {
uv_async_init(uv_default_loop(), &_this->connection_handle,
iotjs_dbus_connection_cb);
}

dbus_connection_set_exit_on_disconnect(_this->connection, false);
Expand Down
13 changes: 13 additions & 0 deletions test/run_pass/test_dbus_connect_with_invalid_addr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

var assert = require('assert');
var dbus = require('dbus');
var start = Date.now();

try {
dbus.getBus();
} catch (err) {
console.log(err.message);
assert.equal(err.message, 'Address does not contain a colon');
}
assert.equal(Date.now() - start < 100, true);
17 changes: 17 additions & 0 deletions test/run_pass/test_dbus_connect_with_nofound.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
'use strict';

var assert = require('assert');
var dbus = require('dbus');

var start = Date.now();
try {
dbus.getBus();
} catch (err) {
console.log(err.message);
assert.equal(err.message,
'Failed to connect to socket /invalid: No such file or directory');
}

var diff = Date.now() - start;
console.log(`consumes ${diff}ms`);
assert.equal(diff > 1000, true);
10 changes: 10 additions & 0 deletions test/testsets.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
{ "name": "test_crypto.js" },
{ "name": "test_crypto_hash.js" },
{ "name": "test_dbus_after_destroy.js" },
{ "name": "test_dbus_connect_with_invalid_addr.js",
"env": {
"DBUS_SESSION_BUS_ADDRESS": "/"
}
},
{ "name": "test_dbus_connect_with_nofound.js",
"env": {
"DBUS_SESSION_BUS_ADDRESS": "unix:path=/invalid"
}
},
{ "name": "test_dbus_define.js" },
{ "name": "test_dbus_introspectable.js" },
{ "name": "test_dbus_send_utf8.js" },
Expand Down
2 changes: 2 additions & 0 deletions tools/testrunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ def run_test(self, testfile, options):
for key, val in options["env"].items():
if key == u"NODE_PATH":
my_env[key] = fs.join(options["root"], val)
elif key == u"DBUS_SESSION_BUS_ADDRESS":
my_env[key] = val

signal.alarm(options["timeout"])

Expand Down

0 comments on commit 259e15a

Please sign in to comment.