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%> (ø)	⬆️

---

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:

go-msgauth/dkim/sign.go

Line 318 in af51aa1

if err := s.Close(); err != nil {

and then also in defer:

go-msgauth/dkim/sign.go

Line 308 in af51aa1

defer s.Close()

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]

Ah, I see. I guess it doesn't matter to return an error in our Close function, because our Write function would have returned an error before.

[emersion]

Thanks!