Skip to content

Commit

Permalink
sbus: terminate ongoing chained requests if backend is restarted
Browse files Browse the repository at this point in the history
If there is an outgoing request already chained and backend is
restarted, a new outgoing request is chained but not processed,
it waits for a timeout.

This patch makes sure that all outgoing requests are gracefully
terminated if backend restarts.

Resolves: #7503
  • Loading branch information
pbrezina committed Oct 7, 2024
1 parent 4295e00 commit 8916cb9
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 4 deletions.
17 changes: 17 additions & 0 deletions src/sbus/connection/sbus_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,20 @@ void sbus_connection_free(struct sbus_connection *conn)
conn);
}
}

void
sbus_connection_terminate_member_requests(struct sbus_connection *conn,
const char *member)
{
DEBUG(SSSDBG_TRACE_FUNC, "Terminating outgoing chained requests for: %s\n",
member);

sbus_requests_terminate_member(conn->requests->outgoing, member,
ERR_TERMINATED);

DEBUG(SSSDBG_TRACE_FUNC, "Terminating incoming chained requests for: %s\n",
member);

sbus_requests_terminate_member(conn->requests->incoming, member,
ERR_TERMINATED);
}
1 change: 1 addition & 0 deletions src/sbus/connection/sbus_dispatcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ sbus_dispatch_reconnect(struct sbus_connection *conn)
/* Terminate all outgoing requests associated with this connection. */
DEBUG(SSSDBG_TRACE_FUNC, "Connection lost. Terminating active requests.\n");
sbus_requests_terminate_all(conn->requests->outgoing, ERR_TERMINATED);
sbus_requests_terminate_all(conn->requests->incoming, ERR_TERMINATED);

switch (conn->type) {
case SBUS_CONNECTION_CLIENT:
Expand Down
5 changes: 5 additions & 0 deletions src/sbus/interface/sbus_std_signals.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ sbus_name_owner_changed(TALLOC_CTX *mem_ctx,
/* Delete any existing sender information since it is now obsolete. */
sbus_senders_delete(conn->senders, name);

/* Terminate active request if the owner has disconnected. */
if (new_owner == NULL || new_owner[0] == '\0') {
sbus_connection_terminate_member_requests(sbus_req->conn, old_owner);
}

return EOK;
}

Expand Down
13 changes: 10 additions & 3 deletions src/sbus/request/sbus_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
DBusMessageIter *write_iter = NULL;
struct sbus_sender *sender;
struct tevent_req *req;
const char *member = NULL;
bool key_exists;
errno_t ret;

Expand All @@ -463,6 +464,9 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
}

state->request->sender = talloc_steal(state->request, sender);
if (sender != NULL) {
member = sender->name;
}

ret = sbus_check_access(state->conn, state->request);
if (ret != EOK) {
Expand Down Expand Up @@ -500,7 +504,7 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
* set a tevent callback that is triggered when the method handler is done.
*/
ret = sbus_requests_add(state->conn->requests->incoming, state->key,
state->conn, req, true, &key_exists);
state->conn, req, member, true, &key_exists);
if (ret != EOK || key_exists) {
/* Cancel the sub request. Since there was either an error or the
* sub request was chained. */
Expand Down Expand Up @@ -617,6 +621,7 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
struct sbus_outgoing_request_state *state;
struct tevent_req *subreq;
struct tevent_req *req;
const char *destination;
bool key_exists;
errno_t ret;

Expand Down Expand Up @@ -645,6 +650,8 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
}
}

destination = dbus_message_get_destination(msg);

/**
* We will search table to see if the same request is not already
* in progress. If it is, we register ourselves for notification
Expand All @@ -654,7 +661,7 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
* set a tevent callback that is triggered when the method handler is done.
*/
ret = sbus_requests_add(conn->requests->outgoing, key,
conn, req, true, &key_exists);
conn, req, destination, true, &key_exists);
if (ret != EOK) {
goto done;
}
Expand Down Expand Up @@ -776,7 +783,7 @@ sbus_request_await_send(TALLOC_CTX *mem_ctx,

/* Otherwise attach to this request. */
ret = sbus_requests_add(conn->requests->outgoing, key, conn,
req, false, NULL);
req, member, false, NULL);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to attach to the request list "
"[%d]: %s\n", ret, sss_strerror(ret));
Expand Down
40 changes: 40 additions & 0 deletions src/sbus/request/sbus_request_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ sbus_requests_add(hash_table_t *table,
const char *key,
struct sbus_connection *conn,
struct tevent_req *req,
const char *member,
bool is_dbus,
bool *_key_exists)
{
Expand Down Expand Up @@ -150,6 +151,11 @@ sbus_requests_add(hash_table_t *table,
item->req = req;
item->conn = conn;
item->is_dbus = is_dbus;
item->member = talloc_strdup(item, member);
if (member != NULL && item->member == NULL) {
ret = ENOMEM;
goto done;
}

ret = sbus_requests_attach_spies(item);
if (ret != EOK) {
Expand Down Expand Up @@ -323,3 +329,37 @@ sbus_requests_terminate_all(hash_table_t *table,

talloc_free(values);
}

void
sbus_requests_terminate_member(hash_table_t *table,
const char *member,
errno_t error)
{
struct sbus_request_list *list;
struct sbus_request_list *item;
hash_value_t *values;
unsigned long int num;
unsigned long int i;
int hret;

hret = hash_values(table, &num, &values);
if (hret != HASH_SUCCESS) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get list of active requests "
"[%d]: %s\n", hret, hash_error_string(hret));
return;
}

for (i = 0; i < num; i++) {
list = sss_ptr_get_value(&values[i], struct sbus_request_list);
if ((member == NULL && list->member == NULL)
|| (member != NULL && list->member != NULL && strcmp(member, list->member) == 0)) {
DLIST_FOR_EACH(item, list) {
sbus_requests_finish(item, error);
}
}

sbus_requests_delete(list);
}

talloc_free(values);
}
10 changes: 10 additions & 0 deletions src/sbus/sbus.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,16 @@ errno_t
sbus_connection_add_path_map(struct sbus_connection *conn,
struct sbus_path *map);

/**
* Terminate all outgoing requests for given member.
*
* @param conn An sbus connection.
* @param member D-Bus member name (destination)
*/
void
sbus_connection_terminate_member_requests(struct sbus_connection *conn,
const char *member);

/**
* Add new signal listener to the router.
*
Expand Down
10 changes: 10 additions & 0 deletions src/sbus/sbus_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ struct sbus_request_list {
struct tevent_req *req;
struct sbus_connection *conn;

/* Member part of the key. Destination for outgoing, sender for incoming.*/
const char *member;

bool is_invalid;
bool is_dbus;

Expand Down Expand Up @@ -462,6 +465,7 @@ sbus_requests_add(hash_table_t *table,
const char *key,
struct sbus_connection *conn,
struct tevent_req *req,
const char *member,
bool is_dbus,
bool *_key_exists);

Expand All @@ -484,6 +488,12 @@ void
sbus_requests_terminate_all(hash_table_t *table,
errno_t error);

/* Terminate requests associated with given member. */
void
sbus_requests_terminate_member(hash_table_t *table,
const char *member,
errno_t error);

/* Create new sbus request. */
struct sbus_request *
sbus_request_create(TALLOC_CTX *mem_ctx,
Expand Down
46 changes: 45 additions & 1 deletion src/tests/system/tests/test_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
from sssd_test_framework.roles.client import Client
from sssd_test_framework.roles.generic import GenericADProvider, GenericProvider
from sssd_test_framework.roles.ipa import IPA
from sssd_test_framework.topology import KnownTopologyGroup
from sssd_test_framework.roles.ldap import LDAP
from sssd_test_framework.topology import KnownTopology, KnownTopologyGroup


@pytest.mark.importance("critical")
Expand Down Expand Up @@ -681,3 +682,46 @@ def test_identity__lookup_when_auto_private_groups_is_set_to_hybrid(client: Clie
assert result is not None, "User 'user_group_gid' not found!"
assert result.gid == 555555, "gid does not match expected value!"
assert client.tools.getent.group(555555) is not None, "auto private group not found!"


@pytest.mark.importance("low")
@pytest.mark.topology(KnownTopology.LDAP)
def test_identity__lookup_when_backend_restarts(client: Client, ldap: LDAP):
"""
:title: Look up user when backend is restarted with previous lookup unfinished
:description:
If there is an active lookup for a user and the backend is restarted
before this lookup is finished, the next lookup of the same user after
the restart must not timeout.
:setup:
1. Add a user "tuser"
2. Start SSSD
:steps:
1. Add 10s network traffic delay to the LDAP host
2. Lookup "tuser" asynchronously
3. Kill sssd_be with SIGKILL so it is restarted
4. Remove the network traffic delay
5. Lookup of "tuser" must yield the user and not timeout
:expectedresults:
1. Network traffic is delayed
2. Lookup hangs, does not finish and waits for a timeout
3. The backend process is restarted
4. Network traffic is no longer delayed
5. User lookup returns the user immediately
:customerscenario: False
"""
ldap.user("tuser").add()

client.sssd.start()

# Add a delay so the next lookup will hang
client.tc.add_delay(ldap, "10s")
client.host.conn.async_run("getent passwd tuser")

# Kill backend and remove the delay
client.host.conn.run("kill -KILL $(pidof sssd_be)")
client.tc.remove_delay(ldap)

# The next lookup should not timeout
result = client.tools.wait_for_condition("getent passwd tuser", timeout=5)
assert "tuser" in result.stdout, "tuser was not found"

0 comments on commit 8916cb9

Please sign in to comment.