Hm, some tests failed, can you please take a look?

Hm, some tests failed, can you please take a look?

@azat the problem is that reply_run_callback() is now accessing handle->base, which is struct evdns_base *base. And test cases free evdns_base while leaving callbacks pending on the queue. For example,

2375     evdns_base_free(env->dns_base, send_err_shutdown);
2376     env->dns_base = 0;
2377     
2378     /* `req` is freed in callback, that's why one loop is required */
2379     event_base_loop(env->base, EVLOOP_NONBLOCK);

evdns_base_free_and_unlock(struct evdns_base *base, int fail_requests) does

4983     /* TODO(nickm) we might need to refcount here. */
4984 
4985     while (base->req_waiting_head) {
4986         if (fail_requests)
4987             reply_schedule_callback(base->req_waiting_head, 0, DNS_ERR_SHUTDOWN, NULL);
4988         request_finished(base->req_waiting_head, &base->req_waiting_head, 1);
4989     }

hm, maybe that's why this "we might need to refcount here" comment is here.

In principal, there is a problem in the existing code too. nameserver_probe_callback(), which is called by reply_run_callback(), already accesses evdns_base through ns->base.

I'll see how to address this. Suggestions are of course welcome.

Updated

On the other hand, there is this code https://github.com/libevent/libevent/blob/master/evdns.c#L5344 handling DNS_ERR_SHUTDOWN case explicitly.

Hmm, how outrageous would it be to schedule a delayed destruction of struct evdns_base *base by the event loop? Like adding a delete-function as the last callback after the ones created by reply_schedule_callback(base->req_heads[i], 0, DNS_ERR_SHUTDOWN, NULL);. Another alternative would be scheduling a single callback that would run reply_run_callback() with DNS_ERR_SHUTDOWN for all requests from req_waiting_queue and req_heads[i] and then free base.

On the other hand, there is this code master/evdns.c#L5344 handling DNS_ERR_SHUTDOWN case explicitly.

All this is too complex...

Hmm, how outrageous would it be to schedule a delayed destruction of struct evdns_base *base by the event loop? Like adding a delete-function as the last callback after the ones created by

Not sure that it is a great idea since it may leave evdns in some corner cases (like event_base_loopexit from some callback), and will require running event_base_loop after evnds_base_free

Another alternative would be scheduling a single callback that would run reply_run_callback() with DNS_ERR_SHUTDOWN for all requests from req_waiting_queue and req_heads[i] and then free base.

This looks like the best way for now to avoid extra complexity for this patch

As a long-term goal I think we need refcnt for evdns_base to avoid use-after-free, we can do it the same way as it is done in bufferevents, decref and free check on unlock and in evdns_base_free.