Skip to content
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

sbus: check connection for NULL before unregister it #232

Closed
wants to merge 1 commit into from

Conversation

sumit-bose
Copy link
Contributor

There seem to be code paths where the data is a added to the hash before
the connection is properly initialized, to avoid core dump during shut
down we only call dbus_conection_unregister_object_path() if there is a
connection.

Resolves https://pagure.io/SSSD/sssd/issue/3367

@pbrezina
Copy link
Member

We should also set dbus.conn to NULL on places where we call dbus_connection_close or dbus_connection_unref.

@@ -490,7 +490,13 @@ sbus_opath_hash_delete_cb(hash_entry_t *item,
conn = talloc_get_type(pvt, struct sbus_connection);
path = sbus_opath_get_base_path(NULL, item->key.str);

dbus_connection_unregister_object_path(conn->dbus.conn, path);
/* There seem to be code paths where the data is a added to the hash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is a added/is added

There seem to be code paths where the data is a added to the hash before
the connection is properly initialized, to avoid core dump during shut
down we only call dbus_conection_unregister_object_path() if there is a
connection.

Resolves https://pagure.io/SSSD/sssd/issue/3367
@sumit-bose
Copy link
Contributor Author

I fixed the typo.

I'm not sure about

We should also set dbus.conn to NULL on places where we call dbus_connection_close or dbus_connection_unref.

Since the connections are ref-counted in libdbus both calls will just modify dbus.conn but not make it unusable for later usage. E.g. if I understand it correctly dbus_connection_close() will not reduce the ref count, so dbus_connection_unref() should always be called on the close connection.
I think it would be a good idea to get rid of the object if it is not needed anymore but it should be carefully checked if it really can be set to NULL or if e.g. there are still other references. Because of this I would prefer to handle this in a different PR. Would you mind to open a ticket about it, so we can come back to this later?

@pbrezina
Copy link
Member

@lslebodn
Copy link
Contributor

lslebodn commented Apr 12, 2017 via email

@lslebodn
Copy link
Contributor

master:

sssd-1-14:

sssd-1-13:

@lslebodn lslebodn closed this Apr 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants