New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

relax calling DBus API without running daemon #1705

Closed
beku opened this Issue Nov 21, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@beku
Member

beku commented Nov 21, 2017

Steps to Reproduce the Problem

Setup a environment without DBus session :/

Expected Result

Continue to work, despite of DBus daemon not running :-)

Actual Result

assert == exit()

System Information

  • Elektra Version: 0.8.20
  • Oyranos:master with active make test-2 target in OBS:oyranos.spec

Further Log Files and Output

OBS with vanilla elektra-0.8.20:
[ 98s] ./test2
[ 98s] process 24035: arguments to dbus_connection_unref() were incorrect, assertion "connection != NULL" failed in file dbus-connection.c line 2823.
[ 98s] This is normally a bug in some application using the D-Bus library.
[ 98s] D-Bus not built with -rdynamic so unable to print a backtrace
[ 99s] ./test2-static
[ 99s] process 24039: arguments to dbus_connection_unref() were incorrect, assertion "connection != NULL" failed in file dbus-connection.c line 2823.
[ 99s] This is normally a bug in some application using the D-Bus library.
[ 99s] D-Bus not built with -rdynamic so unable to print a backtrace
[ 99s] [100%] Test liboyranos extensive.

@beku beku added the bug label Nov 21, 2017

@beku beku added this to the 0.8.21 milestone Nov 21, 2017

@beku beku self-assigned this Nov 21, 2017

@beku

This comment has been minimized.

Show comment
Hide comment
@beku

beku Nov 21, 2017

Member

Elektra tests and those of Oyranos in OBS are solved :-)

Member

beku commented Nov 21, 2017

Elektra tests and those of Oyranos in OBS are solved :-)

@beku beku closed this Nov 21, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Nov 21, 2017

Contributor

Thank you for reporting the issue!

It seems like cf90b7e introduces cleanup at places which are wrong API use.

@waht Do you have time to test+fix the error cases within dbus? Maybe 401b52e already fixes all cases, I did not check.

@beku: Should we do a bug fix release for this problem?

Contributor

markus2330 commented Nov 21, 2017

Thank you for reporting the issue!

It seems like cf90b7e introduces cleanup at places which are wrong API use.

@waht Do you have time to test+fix the error cases within dbus? Maybe 401b52e already fixes all cases, I did not check.

@beku: Should we do a bug fix release for this problem?

@markus2330 markus2330 reopened this Nov 21, 2017

@beku

This comment has been minimized.

Show comment
Hide comment
@beku

beku Nov 21, 2017

Member

To me the dbus_connection_unref (connection) call looks like a clean up from connection = dbus_bus_get (...) . The only case, which is not useful for a dbus_connection_unref(connection) appears to be: if (connection == NULL) ...

So that was maybe a small programming error?

bug fix release?:
I see many warnings from within virtual build/test machines about a missing dbus session. Maybe elektra/dbus and some Xlib related calls have only a problem. But, essentially yes, the main apps shall not crash at all because of such minor issues.

Member

beku commented Nov 21, 2017

To me the dbus_connection_unref (connection) call looks like a clean up from connection = dbus_bus_get (...) . The only case, which is not useful for a dbus_connection_unref(connection) appears to be: if (connection == NULL) ...

So that was maybe a small programming error?

bug fix release?:
I see many warnings from within virtual build/test machines about a missing dbus session. Maybe elektra/dbus and some Xlib related calls have only a problem. But, essentially yes, the main apps shall not crash at all because of such minor issues.

@markus2330 markus2330 assigned waht and unassigned beku Nov 21, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Nov 21, 2017

Contributor

So that was maybe a small programming error?

Yes, it certainly was. Thank you for fixing it!

I see many warnings from within virtual build/test machines about a missing dbus session.

@waht Can you take a look? (Use dbus plugin without dbus session)

One workaround obviously is to unmount the dbus plugin if no dbus daemon is available but I fully agree that there never should be crashes. And we also should avoid the warnings.

Contributor

markus2330 commented Nov 21, 2017

So that was maybe a small programming error?

Yes, it certainly was. Thank you for fixing it!

I see many warnings from within virtual build/test machines about a missing dbus session.

@waht Can you take a look? (Use dbus plugin without dbus session)

One workaround obviously is to unmount the dbus plugin if no dbus daemon is available but I fully agree that there never should be crashes. And we also should avoid the warnings.

@waht

This comment has been minimized.

Show comment
Hide comment
@waht

waht Nov 21, 2017

Contributor

I checked and indeed 401b52e fixed the issue, thank you @beku!

I could not reproduce crashes due to the dbus plugin. I tried kdb set on a mountpoint (with the dbus plugin) in the docker container got the warning, but the key was set and exit code was 0.
Maybe the word "assertion" in the message was misleading as dbus_connection_unref actually just prints a warning and returns in case of connection == NULL.

Currently there are no more warnings when using the dbus plugin without a dbus daemon.
The dbus plugin fails silently unless it was compiled with logging enabled of course.

Contributor

waht commented Nov 21, 2017

I checked and indeed 401b52e fixed the issue, thank you @beku!

I could not reproduce crashes due to the dbus plugin. I tried kdb set on a mountpoint (with the dbus plugin) in the docker container got the warning, but the key was set and exit code was 0.
Maybe the word "assertion" in the message was misleading as dbus_connection_unref actually just prints a warning and returns in case of connection == NULL.

Currently there are no more warnings when using the dbus plugin without a dbus daemon.
The dbus plugin fails silently unless it was compiled with logging enabled of course.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 21, 2017

Contributor

Again thanks for reporting and fixing! The fix will be part of the soon-to-come-release.

Contributor

markus2330 commented Dec 21, 2017

Again thanks for reporting and fixing! The fix will be part of the soon-to-come-release.

@markus2330 markus2330 closed this Dec 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment