dkim: Always close done channel in Sign

[foxcpp]

Added defer close(done) as proposed in the linked issue.

Closes #8.

[codecov-io]

Codecov Report

Merging #15 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   66.46%   66.46%           
=======================================
  Files           7        7           
  Lines         832      832           
=======================================
  Hits          553      553           
  Misses        210      210           
  Partials       69       69

Impacted Files	Coverage Δ
dkim/sign.go	66.88% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 56e7fdd...af51aa1. Read the comment docs.

[emersion]

Not sure why this is necessary. closeReadWithError should make the pipe always return an error?

[foxcpp]

The problem is in dkim.Sign function, in case of signing error, it calls Close twice: https://github.com/emersion/go-msgauth/blob/af51aa1c72014aa9a8c5e867fc3c9bab347d12b4/dkim/sign.go#L318 and then also in defer: https://github.com/emersion/go-msgauth/blob/af51aa1c72014aa9a8c5e867fc3c9bab347d12b4/dkim/sign.go#L308 Second Close reads from done and blocks forever. close(done) fixes it by closing channel so it will return null value without blocking after first read. This makes Close function idempotent (it is safe to call it multiple times), which is also a convenient thing to have.

[emersion]

Sure, but I don't get why s.pw.Close() doesn't return an error in this case?

[foxcpp]

Because it doesn't have to? https://golang.org/pkg/io/#PipeWriter Docs tell that only Write returns an error in case of CloseWithError on read end.

[emersion]

Thanks!