Thread-safe cryptography

Should resolve #157 (closed)

You can do what you described in the comment here. Just move the event cancellation to the beginning of the block so you can be sure it won't be triggered during the execution. Also, there is another problem (probably the crash) I see the zones_dnssec_ev free the old pointer and so does the zones_cancel_dnssec. You should reuse the event and free it only when finally terminated because following may happen:

(evsched, end of the function)zones_dnssec_ev:
evsched_event_free(event->parent, event); # zd->dnssec_timer is now freed
zd->dnssec_timer = NULL; # zd->dnssec_timer is now NULL
zones_schedule_dnssec() # zd->dnssec_timer is now valid event

(some other thread)zones_cancel_dnssec:
evsched_cancel(scheduler, zd->dnssec_timer); # zd->dnssec_timer might be freed (crash), NULL (noop), valid (okay)
evsched_event_free(scheduler, zd->dnssec_timer); # the event now cancelled, but it might have made it to reschedule so zd->dnssec_timer might be something else than was cancelled (so we've just freed a running event without cancellation)
zd->dnssec_timer = NULL;

If you reuse the same event, the worst thing that could happen is that you attempted to cancel it, but it got rescheduled. This is easily identified by evsched_cancel() that could be made to return different value (evsched.c:328) if it couldn't cancel a running event. If it couldn't cancel a running event, then you would need to call evsched_cancel() again until it returns positive result.

The first part of this comment I understand - good idea, I'll do that. As for the second part - that was exactly why I wanted to lock these operations using the zd lock. The crash was something else though, probably not related.

The change seems to have resolved the issue, but there are some other problems that became visible.

And are the other problems fixed now? Should I review your patches?

To merge, or not to merge, that is the question.

This will cause infinite loop if invalid parameters are supplied to evsched_cancel. I think evsched_cancel should return a different result code for error and for rescheduled events. And the loop should checked just for the one error code.

At this point, we are sure that the parameters are fine. I'll slap few asserts there to be sure. I think the evsched_cancel function should not return until the event, and the event planned by the original event are both cancelled. because I am certainly against checking for -2 or something like that (since evsched.c does not use Knot error codes).

I've created a new API function in evsched, that handles the planned events as well, @mvavrusa please review and change it if needed. It's 469e5086

I'm OK with asserts.

I'll create a new merge request with additional changes, this got out of hand.

Status changed to closed

mentioned in merge request !109 (merged)