Fix DNS resolve mutex locks

[sarchar]

This fixes #49261, which was happening because of a deadlock in the resolver mutex. There was leftover old mutex code and it's all be converted to new MutexLock class now.

[akien-mga]

There's still three more calls to unlock() in early returns, shouldn't they be removed?

[sarchar]

There's still three more calls to unlock() in early returns, shouldn't they be removed?

Yeah, I think they probably should be, but they were in the code before my original PR and so I was skeptical to touch them in case someone else added them for some actual purpose.

Should I remove them too? They seem to all be in error condition branches that probably never get hit, which is why they've not been a problem.

[akien-mga]

Right, this might need checking by @RandomShaper who implemented the Mutex changes in 18fbdbb. In this commit he removed most ->unlock() calls but he kept two of them as .unlock(), and then your PR added a third one following the same pattern.

[sarchar]

Right, this might need checking by @RandomShaper who implemented the Mutex changes in 18fbdbb. In this commit he removed most ->unlock() calls but he kept two of them as .unlock(), and then your PR added a third one following the same pattern.

OK, so wait and see what @RandomShaper says. I can easily remove them if he says they're not needed.

[RandomShaper]

Indeed, I took things out of balance by adding the RAII (MutexLock) style and forgetting to remove some of the explicit unlocks.

By the way, this PR makes the same mistake. 😃 Please remove the explicit unlocks inside the ifs where you have switched to the scoped lock style, and please do so also in get_resolve_item_status(), where I forgot myself.

Furthermore, I don't think it will actually impact performance, but it's even better to enclose each MutexLock declaration along with the code it protects in a scope so the lock is released earlier instead of at the end of the function. I don't know if the original intent of the code regarding the early-unlock was optimization or just to avoid writing the unlock call many times, but, in any case, it won't harm.

UPDATE: To elaborate, I didn't have a strong reason to keep the explicit lock/unlock or fully switch to the scoped style. I just decided on a per case basis judging which would be more readable, but now I think it's better to use MutexLock everywhere, at least in this class, for consistency.

[sarchar]

By the way, this PR makes the same mistake. 😃 Please remove the explicit unlocks inside the ifs where you have switched to the scoped lock style, and please do so also in get_resolve_item_status(), where I forgot myself.

In that case, I'll remove the extra unlock() calls and push an update in 10 minutes.

[akien-mga]

Thanks!