Skip to content

searchAsync checks cancelled context only after receiving an ldap response #495

@SaschaRoland

Description

@SaschaRoland

I have a use case with long lasting ldap requests to retrieve many single entries from a ldap server.
To stay responsive for application shutdown, ... during such a query, searchAsync is used with a Context, that will be cancelled once the query has to be terminated early.

This works in principle, but the ldap server also shows the behaviour that it can take a long period of time (minutes) between the returned sinlge search results.
Unfortunately, cancelling the async search is also blocked during such periods, resp. the cancelled context will not be recognized until some event from the server side occurs.

Regarding the implementation in response.go, this is probably caused by waiting for new responses via the default case in the according select statement:

func (r *searchResponse) start(ctx context.Context, searchRequest *SearchRequest)
	go func() {
        ....
		foundSearchSingleResultDone := false
		for !foundSearchSingleResultDone {
			select {
			case <-ctx.Done():
				r.conn.Debug.Printf("%d: %s", msgCtx.id, ctx.Err().Error())
				return
			default:
				r.conn.Debug.Printf("%d: waiting for response", msgCtx.id)
				packetResponse, ok := <-msgCtx.responses
        ....    

So, the check for ctx.Done() only happens between blocking reads on msgCtx.responses.

I guess, handling new repsonses in a distinct case would solve this problem:

func (r *searchResponse) start(ctx context.Context, searchRequest *SearchRequest)
	go func() {
        ....
		foundSearchSingleResultDone := false
		for !foundSearchSingleResultDone {
   		        r.conn.Debug.Printf("%d: waiting for response", msgCtx.id)
			select {
			case <-ctx.Done():
				r.conn.Debug.Printf("%d: %s", msgCtx.id, ctx.Err().Error())
				return
			case packetResponse, ok := <-msgCtx.responses:
        ....    

As far as I can see, there is no reason against using a specific case, but I'm far from having sufficient overview of the code.
Could someone explain, whether there is a need for the default case?
Many thanks in advance.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions