On Thu, 21 Mar 2019 at 16:14, <jake6...@gmail.com> wrote: > Two things to note about this. > > First, if the "everything else" deferred to might include a return > statement, then this could result in the same "silent failure" behavior. >
Yup, don't do that :) > Secondly - use this pattern with great caution. It is critical to confirm > that a double close on ifDB is explicitly allowed. For example, if you have > an io.Closer <https://golang.org/pkg/io/#Closer>, then, according to the > documentation, the behavior of a double close is not specified. IMHO, if > the package documentation does not explicitly allow double close, I would > consider doing so to to be a bug. > Yup, this is definitely true. I've seen implementations of Close which just decrement a reference count, which makes it really fragile! For the record, another idiom I've used looks something like this: func doSomething() (err error) { conn, err := openConn() if err != nil { return err } defer checkClose(&err, conn) if err := doStuff(conn); err != nil { return err } return nil } // checkClose closes c and sets *err to any resulting error if it's not // already non-nil. func checkClose(err *error, c io.Closer) { closeErr := c.Close() if *err != nil { // Could log the close error here. return } *err = fmt.Errorf("cannot close: %v", closeErr) } This works well if you're doing it a lot, but is quite vulnerable to variable-shadowing bugs, so best used with care in a highly stylized manner. > > Only do this if you can confirm that a double close on ifDB > > On Thursday, March 21, 2019 at 6:11:04 AM UTC-4, rog wrote: >> >> This issue is why I like having idempotent close. Do: >> >> defer ifDB.Close() >> >> but also, after everything else is complete, do: >> >> if err := ifDB.Close(); err != nil { >> handle error case >> } >> >> That way if you return early on error your connection is still closed, >> but you can still write the obvious code for checking the close error when >> everything else has succeeded. >> >> >> On Thu, 21 Mar 2019 at 09:41, Amnon Baron Cohen <amn...@gmail.com> wrote: >> >>> The idiomatic Go way is to write >>> >>> defer ifDB.Close() >>> >>> Simple is better than clever. >>> >>> + Readability does matter. >>> >>> >>> And in situations where you really need to check that the connection was >>> successfully closed, and >>> you have a workable scheme for recovering from this situation, then >>> write a named helper function >>> which encompasses this logic, rather than writing a convoluted anonymous >>> function. >>> >>> And make sure that all the execution paths in this function have unit >>> tests. >>> >>> >>> >>> On Tuesday, 19 March 2019 23:57:51 UTC, David Collier-Brown wrote: >>>> >>>> It's a known bad thing to defer a close for anything buffered, as >>>> discussed at >>>> https://www.joeshaw.org/dont-defer-close-on-writable-files/ >>>> but some constructs lack a Sync() call. >>>> >>>> For example, I have code like >>>> ifDB, message, err := OpenInflux(server, debug) >>>> if err != nil { >>>> // If this fails, fail fast so we notice >>>> panic(fmt.Errorf("failed to open and ping influxDB, %s >>>> %v", >>>> message, err)) >>>> } >>>> defer func() { >>>> err = ifDB.Close() >>>> if err != nil { >>>> // This may be a harmless, very bad thing >>>> panic(fmt.Errorf("failure closing connection to >>>> influxDB, %v", err)) >>>> } >>>> }() >>>> >>>> >>>> which closes an influxDB database, but will panic if the close fails. >>>> >>>> Separate logic makes sure I have passed the point at which the data is >>>> committed before I commit my reading of input, so eventually the data will >>>> be reread and rewritten. Nevertheless, this is exceedingly complex, and the >>>> comment sums it up: "a harmless, very bad thing". >>>> >>>> Is there an idiomatic way to address this? I ended up reading influxDB >>>> code and doing all sorts of deranged safety-critical-system DFAs to >>>> reassure myself this will actually work, but every time I do that, my brain >>>> hurts! >>>> >>>> --dave >>>> >>>> >>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "golang-nuts" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to golang-nuts...@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
