using defer inside for loop could lead to leaks

[halilylm]

you create resources inside for loop and defer inside for loop, the defer function will execute after function returns, so you won't be able to clean resources inside for loop, it all will wait until function exists. I put close after for loop ended. so it will release the resources after for loop ends.

[wneessen]

Thanks for the PR @halilylm. I was thinking about this already when I ported it from the original net/smtp but left it for now. But it's probably a good idea to adress this directly. It looks like the tests on smtp are failing now though, since we are focing the close on something that might not always be "closeable". Can you check if you can make the tests working again? Otherwise I suggest to leave it as is since we are "only" talking about tests here.

[james-d-elliott]

You can fix this most likely another way by placing both the defer and the NewClient() (and anything that uses the client) calls within a function (anonymous or named) inside the for loop most likely; as the issue with for loops and defer are that defer executes at the end of the function call not the for loop.

[halilylm]

Sorry I didn't have time to fix errors to pass tests that's why I put in draft. @james-d-elliott yes wrapping in a function could solve it, we can call defer in that function without waiting all for loop.

[wneessen]

@halilylm following up on this, do you think you'll have time to complete this or should I go ahead and work on a fix? I'm planning to release the next go-mail version soon and I think this would be a good addition to be added to the release. Let me know.

[halilylm]

@wneessen hey sorry, I am too busy to make it. I'll leave it to you

[wneessen]

@wneessen hey sorry, I am too busy to make it. I'll leave it to you

All good. Thanks for the update and brining up this issue.

[wneessen]

Superseded by #122