Make WINBINDD_LIST_GROUPS handler asynchronous.

Previously WINBINDD_LIST_GROUPS requests (ex: wbinfo -g) were handled by the
winbindd parent process in a sequential fashion.  This patch, delegates the work
to the winbindd children so that the request is handled much faster in large
domain topologies, and doesn't block the parent from receiving new requests.

The core group enumeration and conversion that was handled in
winbindd_list_groups() has been moved into winbindd_dual_list_groups() to be
done by the child.

The parent winbindd_list_groups() simply calls each of the children
asynchronously.

listgroups_recv() aggregates the final group list that will be returned to the
client and tracks how many of the children have returned their lists.

The domain name of the child is passed back through the callbacks to be used in
debugging messages.

There are also several fixes to typos in various comments.
(This used to be commit 037b9689d9042a398cb91e4628a82fcdfa913c21)

6 files changed, 187 insertions, 60 deletions

diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index 8be6a47998..5e3d5d2aec 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -393,7 +393,7 @@ static NTSTATUS enum_local_groups(struct winbindd_domain *domain,
 	 * using LDAP.
 	 *
 	 * if we ever need to enumerate domain local groups separately, 
-	 * then this the optimization in enum_dom_groups() will need 
+	 * then this optimization in enum_dom_groups() will need
 	 * to be split out
 	 */
 	*num_entries = 0;
diff --git a/source3/winbindd/winbindd_async.c b/source3/winbindd/winbindd_async.c
index 2ff5ef230d..635bc6b244 100644
--- a/source3/winbindd/winbindd_async.c
+++ b/source3/winbindd/winbindd_async.c
@@ -453,6 +453,97 @@ enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain,
 	return WINBINDD_OK;
 }
 
+/* This is the first callback after enumerating groups from a domain */
+static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success,
+	                    struct winbindd_response *response,
+			    void *c, void *private_data)
+{
+	void (*cont)(void *priv, bool succ, fstring dom_name, char *data) =
+		(void (*)(void *, bool, fstring, char*))c;
+
+	if (!success || response->result != WINBINDD_OK) {
+		DEBUG(5, ("list_groups() failed!\n"));
+		cont(private_data, False, response->data.name.dom_name, NULL);
+		return;
+	}
+
+	cont(private_data, True, response->data.name.dom_name,
+	     response->extra_data.data);
+
+	SAFE_FREE(response->extra_data.data);
+}
+
+/* Request the name of all groups in a single domain */
+void winbindd_listgroups_async(TALLOC_CTX *mem_ctx,
+	                       struct winbindd_domain *domain,
+	                       void (*cont)(void *private_data, bool success,
+				     fstring dom_name, char* extra_data),
+			       void *private_data)
+{
+	struct winbindd_request request;
+
+	ZERO_STRUCT(request);
+	request.cmd = WINBINDD_LIST_GROUPS;
+
+	do_async_domain(mem_ctx, domain, &request, listgroups_recv,
+		        (void *)cont, private_data);
+}
+
+enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain,
+                                               struct winbindd_cli_state *state)
+{
+	struct getent_state groups = {};
+	char *extra_data = NULL;
+	unsigned int extra_data_len = 0, i;
+
+	/* Must copy domain into response first for bookeeping in parent */
+	fstrcpy(state->response.data.name.dom_name, domain->name);
+	fstrcpy(groups.domain_name, domain->name);
+
+	/* Get list of sam groups */
+	if (!get_sam_group_entries(&groups)) {
+		/* this domain is empty or in an error state */
+		return WINBINDD_ERROR;
+	}
+
+	/* Allocate some memory for extra data.  Note that we limit
+	   account names to sizeof(fstring) = 256 characters.
+	   +1 for the ',' between group names */
+	extra_data = (char *)SMB_REALLOC(extra_data,
+		(sizeof(fstring) + 1) * groups.num_sam_entries);
+
+	if (!extra_data) {
+		DEBUG(0,("failed to enlarge buffer!\n"));
+		SAFE_FREE(groups.sam_entries);
+		return WINBINDD_ERROR;
+	}
+
+	/* Pack group list into extra data fields */
+	for (i = 0; i < groups.num_sam_entries; i++) {
+		char *group_name = ((struct acct_info *)
+				    groups.sam_entries)[i].acct_name;
+		fstring name;
+
+		fill_domain_username(name, domain->name, group_name, True);
+		/* Append to extra data */
+		memcpy(&extra_data[extra_data_len], name, strlen(name));
+		extra_data_len += strlen(name);
+		extra_data[extra_data_len++] = ',';
+	}
+
+	SAFE_FREE(groups.sam_entries);
+
+	/* Assign extra_data fields in response structure */
+	if (extra_data) {
+		/* remove trailing ',' */
+		extra_data[extra_data_len - 1] = '\0';
+		state->response.extra_data.data = extra_data;
+		state->response.length += extra_data_len;
+	}
+
+	return WINBINDD_OK;
+}
+
 bool print_sidlist(TALLOC_CTX *mem_ctx, const DOM_SID *sids,
 		   size_t num_sids, char **result, ssize_t *len)
 {
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 020338165b..f0ff0294f3 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -524,7 +524,7 @@ static void refresh_sequence_number(struct winbindd_domain *domain, bool force)
 	domain->last_status = status;
 	domain->last_seq_check = time(NULL);
 	
-	/* save the new sequence number ni the cache */
+	/* save the new sequence number in the cache */
 	store_cache_seqnum( domain );
 
 done:
diff --git a/source3/winbindd/winbindd_domain.c b/source3/winbindd/winbindd_domain.c
index 1b758cdf40..4d10f49ca2 100644
--- a/source3/winbindd/winbindd_domain.c
+++ b/source3/winbindd/winbindd_domain.c
@@ -50,6 +50,10 @@ static const struct winbindd_child_dispatch_table domain_dispatch_table[] = {
 		.struct_cmd	= WINBINDD_LOOKUPRIDS,
 		.struct_fn	= winbindd_dual_lookuprids,
 	},{
+		.name		= "LIST_GROUPS",
+		.struct_cmd	= WINBINDD_LIST_GROUPS,
+		.struct_fn	= winbindd_dual_list_groups,
+	},{
 		.name		= "LIST_TRUSTDOM",
 		.struct_cmd	= WINBINDD_LIST_TRUSTDOM,
 		.struct_fn	= winbindd_dual_list_trusted_domains,
diff --git a/source3/winbindd/winbindd_group.c b/source3/winbindd/winbindd_group.c
index 63fde9f495..dd2fc6f6b5 100644
--- a/source3/winbindd/winbindd_group.c
+++ b/source3/winbindd/winbindd_group.c
@@ -971,10 +971,9 @@ void winbindd_endgrent(struct winbindd_cli_state *state)
 
 /* Get the list of domain groups and domain aliases for a domain.  We fill in
    the sam_entries and num_sam_entries fields with domain group information.  
-   The dispinfo_ndx field is incremented to the index of the next group to 
-   fetch. Return True if some groups were returned, False otherwise. */
+   Return True if some groups were returned, False otherwise. */
 
-static bool get_sam_group_entries(struct getent_state *ent)
+bool get_sam_group_entries(struct getent_state *ent)
 {
 	NTSTATUS status;
 	uint32 num_entries;
@@ -1340,15 +1339,23 @@ void winbindd_getgrent(struct winbindd_cli_state *state)
 		request_error(state);
 }
 
-/* List domain groups without mapping to unix ids */
+struct listgroups_state {
+	TALLOC_CTX *mem_ctx;
+	struct winbindd_cli_state *cli_state;
+	unsigned int domain_count;
+	char *extra_data;
+	unsigned int extra_data_len;
+};
 
+static void listgroups_recv(void *private_data, bool success, fstring dom_name,
+	                    char *extra_data);
+
+/* List domain groups without mapping to unix ids */
 void winbindd_list_groups(struct winbindd_cli_state *state)
 {
-	uint32 total_entries = 0;
 	struct winbindd_domain *domain;
 	const char *which_domain;
-	char *extra_data = NULL;
-	unsigned int extra_data_len = 0, i;
+	struct listgroups_state *groups_state;
 
 	DEBUG(3, ("[%5lu]: list groups\n", (unsigned long)state->pid));
 
@@ -1356,74 +1363,87 @@ void winbindd_list_groups(struct winbindd_cli_state *state)
 	state->request.domain_name[sizeof(state->request.domain_name)-1]='\0';	
 	which_domain = state->request.domain_name;
 	
-	/* Enumerate over trusted domains */
+	/* Initialize listgroups_state */
+	groups_state = TALLOC_P(state->mem_ctx, struct listgroups_state);
+	if (groups_state == NULL) {
+		DEBUG(0, ("talloc failed\n"));
+		request_error(state);
+		return;
+	}
 
-	for (domain = domain_list(); domain; domain = domain->next) {
-		struct getent_state groups;
+	groups_state->mem_ctx = state->mem_ctx;
+	groups_state->cli_state = state;
+	groups_state->domain_count = 0;
+	groups_state->extra_data = NULL;
+	groups_state->extra_data_len = 0;
 
+	/* Must count the full list of expected domains before we request data
+	 * from any of them.  Otherwise it's possible for a connection to the
+	 * first domain to fail, call listgroups_recv(), and return to the
+	 * client without checking any other domains. */
+	for (domain = domain_list(); domain; domain = domain->next) {
 		/* if we have a domain name restricting the request and this
 		   one in the list doesn't match, then just bypass the remainder
 		   of the loop */
-		   
 		if ( *which_domain && !strequal(which_domain, domain->name) )
 			continue;
-			
-		ZERO_STRUCT(groups);
 
-		/* Get list of sam groups */
-		
-		fstrcpy(groups.domain_name, domain->name);
-
-		get_sam_group_entries(&groups);
-			
-		if (groups.num_sam_entries == 0) {
-			/* this domain is empty or in an error state */
-			continue;
-		}
+		groups_state->domain_count++;
+	}
 
-		/* keep track the of the total number of groups seen so 
-		   far over all domains */
-		total_entries += groups.num_sam_entries;
-		
-		/* Allocate some memory for extra data.  Note that we limit
-		   account names to sizeof(fstring) = 128 characters.  */		
-		extra_data = (char *)SMB_REALLOC(
-			extra_data, sizeof(fstring) * total_entries);
- 
-		if (!extra_data) {
-			DEBUG(0,("failed to enlarge buffer!\n"));
-			request_error(state);
-			return;
-		}
+	/* Make sure we're enumerating at least one domain */
+	if (!groups_state->domain_count) {
+		request_ok(state);
+		return;
+	}
 
-		/* Pack group list into extra data fields */
-		for (i = 0; i < groups.num_sam_entries; i++) {
-			char *group_name = ((struct acct_info *)
-					    groups.sam_entries)[i].acct_name; 
-			fstring name;
-
-			fill_domain_username(name, domain->name, group_name, True);
-			/* Append to extra data */			
-			memcpy(&extra_data[extra_data_len], name, 
-                               strlen(name));
-			extra_data_len += strlen(name);
-			extra_data[extra_data_len++] = ',';
-		}
+	/* Enumerate list of trusted domains and request group list from each */
+	for (domain = domain_list(); domain; domain = domain->next) {
+		if ( *which_domain && !strequal(which_domain, domain->name) )
+			continue;
 
-		SAFE_FREE(groups.sam_entries);
+		winbindd_listgroups_async(state->mem_ctx, domain,
+			                  listgroups_recv, groups_state);
 	}
+}
+
+static void listgroups_recv(void *private_data, bool success, fstring dom_name,
+	                    char *extra_data)
+{
+	/* extra_data comes to us as a '\0' terminated string of comma
+	   separated groups */
+	struct listgroups_state *state = private_data;
 
-	/* Assign extra_data fields in response structure */
+	/* Append groups from one domain onto the whole list */
 	if (extra_data) {
-		extra_data[extra_data_len - 1] = '\0';
-		state->response.extra_data.data = extra_data;
-		state->response.length += extra_data_len;
+		DEBUG(5, ("listgroups_recv: %s returned groups.\n", dom_name));
+		if (!state->extra_data)
+			state->extra_data = talloc_asprintf(state->mem_ctx,
+							    "%s", extra_data);
+		else
+			state->extra_data = talloc_asprintf_append_buffer(
+							    state->extra_data,
+							    ",%s", extra_data);
+		/* Add one for the '\0' and each additional ',' */
+		state->extra_data_len += strlen(extra_data) + 1;
+	}
+	else {
+		DEBUG(5, ("listgroups_recv: %s returned no groups.\n",
+			dom_name));
 	}
 
-	/* No domains may have responded but that's still OK so don't
-	   return an error. */
+	if (--state->domain_count)
+		/* Still waiting for some child domains to return */
+		return;
 
-	request_ok(state);
+	/* Return list of all groups to the client */
+	if (state->extra_data) {
+		state->cli_state->response.extra_data.data =
+			SMB_STRDUP(state->extra_data);
+		state->cli_state->response.length += state->extra_data_len;
+	}
+
+	request_ok(state->cli_state);
 }
 
 /* Get user supplementary groups.  This is much quicker than trying to
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 8ec6f7e108..f6a0da8fff 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -133,6 +133,16 @@ void query_user_async(TALLOC_CTX *mem_ctx, struct winbindd_domain *domain,
 				   uint32 group_rid),
 		      void *private_data);
 
+void winbindd_listgroups_async(TALLOC_CTX *mem_ctx,
+	                       struct winbindd_domain *domain,
+	                       void (*cont)(void *private_data, bool success,
+				     fstring dom_name, char* extra_data),
+			       void *private_data);
+
+enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain,
+                                               struct winbindd_cli_state *state);
+
+
 /* The following definitions come from winbindd/winbindd_cache.c  */
 
 void winbindd_check_cache_size(time_t t);
@@ -330,6 +340,8 @@ void winbindd_getusersids(struct winbindd_cli_state *state);
 void winbindd_getuserdomgroups(struct winbindd_cli_state *state);
 enum winbindd_result winbindd_dual_getuserdomgroups(struct winbindd_domain *domain,
 						    struct winbindd_cli_state *state);
+bool get_sam_group_entries(struct getent_state *ent);
+
 
 /* The following definitions come from winbindd/winbindd_idmap.c  */