From 259e15a6a98b3860a099704a20c1ba24a0ec2882 Mon Sep 17 00:00:00 2001 From: Yorkie Liu Date: Thu, 8 Nov 2018 23:47:34 +0800 Subject: [PATCH] dbus: add an external param `waitOnDelay` to handle service unavailable 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". --- src/js/dbus.js | 12 ++--- src/modules/iotjs_module_dbus.c | 45 ++++++++++++++----- .../test_dbus_connect_with_invalid_addr.js | 13 ++++++ .../test_dbus_connect_with_nofound.js | 17 +++++++ test/testsets.json | 10 +++++ tools/testrunner.py | 2 + 6 files changed, 84 insertions(+), 15 deletions(-) create mode 100644 test/run_pass/test_dbus_connect_with_invalid_addr.js create mode 100644 test/run_pass/test_dbus_connect_with_nofound.js diff --git a/src/js/dbus.js b/src/js/dbus.js index ceb0807ae..b5df58927 100644 --- a/src/js/dbus.js +++ b/src/js/dbus.js @@ -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; } @@ -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; } diff --git a/src/modules/iotjs_module_dbus.c b/src/modules/iotjs_module_dbus.c index ff75d2631..1a568adf3 100644 --- a/src/modules/iotjs_module_dbus.c +++ b/src/modules/iotjs_module_dbus.c @@ -2,6 +2,7 @@ #include "iotjs_objectwrap.h" #include #include +#include typedef struct { iotjs_jobjectwrap_t jobjectwrap; @@ -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); \ @@ -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(); } @@ -423,15 +422,38 @@ 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)) { @@ -439,6 +461,9 @@ JS_FUNCTION(GetBus) { } 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); diff --git a/test/run_pass/test_dbus_connect_with_invalid_addr.js b/test/run_pass/test_dbus_connect_with_invalid_addr.js new file mode 100644 index 000000000..1acc3951b --- /dev/null +++ b/test/run_pass/test_dbus_connect_with_invalid_addr.js @@ -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); diff --git a/test/run_pass/test_dbus_connect_with_nofound.js b/test/run_pass/test_dbus_connect_with_nofound.js new file mode 100644 index 000000000..5f6dba2bb --- /dev/null +++ b/test/run_pass/test_dbus_connect_with_nofound.js @@ -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); diff --git a/test/testsets.json b/test/testsets.json index c7c42f16c..0aef9fbdd 100644 --- a/test/testsets.json +++ b/test/testsets.json @@ -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" }, diff --git a/tools/testrunner.py b/tools/testrunner.py index c709752ec..1f6c984d0 100755 --- a/tools/testrunner.py +++ b/tools/testrunner.py @@ -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"])