-
Notifications
You must be signed in to change notification settings - Fork 367
Description
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.