Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap closing order in inAxfr and inIxfr #1511

Merged
merged 2 commits into from Nov 13, 2023

Conversation

noxer
Copy link
Contributor

@noxer noxer commented Nov 13, 2023

This PR swaps the order in which the connections and channels are closed in inAxfr and inIxfr. This makes the order more logical (consumers of the channel can be sure that the connection has been closed when the channel is closed).

The motivation for changing this is a heisenbug we had in our test pipeline. After closing the channel, the test code checked if the connection had been closed as well. Due to the nature of go routines, the test would randomly fail, depending on the scheduler. Swapping the order eliminates the problem, putting both deferred calls into the same function may even give (a very slight) performance improvement and it makes the order of operations more predictable and logical.

If you have any questions or suggestions I'm happy to incorporate them.

-- Tim

@miekg
Copy link
Owner

miekg commented Nov 13, 2023

thanks, can you add a comment in the code (at both places) where you briefly summarize why this is done in this (new) order? thanks!

@noxer
Copy link
Contributor Author

noxer commented Nov 13, 2023

@miekg thank you for your feedback. I've added the comments.

@miekg miekg merged commit 3d593a6 into miekg:master Nov 13, 2023
4 checks passed
@noxer noxer deleted the fix-closing-order-in-inAxfr-and-inIxfr branch November 13, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants