31 ms·

I write a lot of Go, and I agree that this is a big wart in its error handling that would be served by a proper Result type. Nevertheless, the convention is th

I write a lot of Go, and I agree that this is a big wart in its error handling that would be served by a proper Result type. Nevertheless, the convention is that if a function returns (value, err), and err != nil, the value is discarded (I think of it as "undefined"). So the code is conventional.

amluto 4y ago
In C, “discarding” a pointer in a way that leaves the value visible is quite common. At least if one doesn’t accidentally use the pointer, it’s harmless. (In the way that all manner of unsafeness is harmless in C as long as no actual UB occurs, which is to say it’s not great.) But Go is a garbage collected language, and there is so such thing as “discarding” a pointer. Either it’s there or it isn’t, and this kind of leak has side effects. I find it baffling that the language designers and the community consider this acceptable. (One thing I really like about Rust is that you can’t play fast and loose with lifetimes like this. If you have function taking &'a Vec<T> and you return &'a T, you can’t arbitrarily “discard” and leak that reference up the call chain. You need to genuinely get rid of it by the time the vector is gone.)
- foldr 4y ago
  >and this kind of leak has side effects Only if the calling function does something with the pointer (which it generally won't, if err is non-nil). If the calling code does do something with the pointer even when err is non-nil, then either (a) the value of the pointer is meaningful, and this is fine; or (b) there's a logic error in the calling code, which is a far more serious bug than a potential memory leak. So I don't really see the problem here.
  - amluto 4y ago
    I should have been more explicit. Go is garbage collected. If you keep a pointer live, then it’s not garbage, which prevents it from being collected. At least Go doesn’t use RAII, so inadvertently pinning a pointer won’t keep a socket open or a lock held.
    - foldr 4y ago
      Just returning the pointer doesn't keep the value that it points to live, unless the calling function does something with the pointer. The following toy example doesn't leak memory. Are you thinking of some other pattern that would leak memory? If so, what is it? I can only think of patterns where a logic error is also involved. That is, where the meaningless value of 'val' is used in code paths where err != nil. func alwaysError() (*int, error) { var dummy int // We return &dummy even though it's a meaningless value. // Will this cause a memory leak? return &dummy, fmt.Errorf("An error") } func caller() { val, err := alwaysError() if err != nil { fmt.Printf("Error\n") // It won't! Because the value pointed to by 'val' // can be GCed from this point on. return } // never get here fmt.Printf("Value %v\n", val) // // ...lots of code that uses *val... // } You might think that you'd get a leak if the error branch was also long-lived, but Go's GC seems to be precise with respect to conditional branching (as far as I can tell by experimenting with runtime.SetFinalizer). That is, as long as the error branch doesn't refer to 'val', then the value pointed to by 'val' can be collected once the error branch is entered (and before 'caller' returns).
      - amluto 4y ago
        I haven't actually played with this, but if the same pattern of lazily (sloppily?) returning arbitrary pointers along with errors continues, the lifetime ought to be extended arbitrarily. The caller could itself return val, err, and so on up the chain.
        foldr 4y ago
        I'm not seeing how that would cause a memory leak. It just means that the value might be GCed a bit later. Could you give an example?
        amluto 4y ago
        It’s a temporary leak. My point is that it’s an actual side effect, unlike how in C it has no effect whatsoever if the value is ignored. In a language where destruction of an object often releases external resources (C++, for example, or Python or Rust to a lesser extent), this effect would be more dramatic. Imagine someone doing this in Python: def func(): f = tmpfile(…) err = do something return f, err This style would be absurd and wrong — it keeps f alive to long.
za3faran 4y ago
What I found frustrating that there is nothing stopping a function from returning both a non nil value and a non nil error. I've seen weird code that did that and it could have easily resulted in incorrect behavior.
- klodolph 4y ago
  The one place where you’ll actually see this is in Read(). If you try to read 1000 bytes, you might get “800 bytes read, EOF” as a result. The worst part is that os.File won’t ever do this!
  - ok_dad 4y ago
    I really hate when people use "errors" for signals. Or, alternately, use the signals mechanisms for actual errors. An error should be "something went wrong", and reading a file to the EOF is not "going wrong", that's just what the "read()" should do if you tell it so! I like Go's multiple returns and error checking by default, but it definitely should have been implemented with some sort of "Result/Error" type union instead. Go made so many good decisions, that I am frankly amazed they made this one really bad one.
    - robotresearcher 4y ago
      If you take the position that the semantics of read() are 'read data from this endless stream', then EOF is an error indicating that this model no longer applies. Doesn't bother me at all.
      - account42 4y ago
        Except in that model it would be fine to return an error as soon as soon as you know there is an EOF while in the real world you do want to read right up to the EOF because EOF is usually not an error condition but expected, even with streams of indeterminate length.
        4y ago

amluto 4y ago

In C, “discarding” a pointer in a way that leaves the value visible is quite common. At least if one doesn’t accidentally use the pointer, it’s harmless. (In the way that all manner of unsafeness is harmless in C as long as no actual UB occurs, which is to say it’s not great.) But Go is a garbage collected language, and there is so such thing as “discarding” a pointer. Either it’s there or it isn’t, and this kind of leak has side effects. I find it baffling that the language designers and the community consider this acceptable. (One thing I really like about Rust is that you can’t play fast and loose with lifetimes like this. If you have function taking &'a Vec<T> and you return &'a T, you can’t arbitrarily “discard” and leak that reference up the call chain. You need to genuinely get rid of it by the time the vector is gone.)

foldr 4y ago
>and this kind of leak has side effects Only if the calling function does something with the pointer (which it generally won't, if err is non-nil). If the calling code does do something with the pointer even when err is non-nil, then either (a) the value of the pointer is meaningful, and this is fine; or (b) there's a logic error in the calling code, which is a far more serious bug than a potential memory leak. So I don't really see the problem here.
- amluto 4y ago
  I should have been more explicit. Go is garbage collected. If you keep a pointer live, then it’s not garbage, which prevents it from being collected. At least Go doesn’t use RAII, so inadvertently pinning a pointer won’t keep a socket open or a lock held.
  - foldr 4y ago
    Just returning the pointer doesn't keep the value that it points to live, unless the calling function does something with the pointer. The following toy example doesn't leak memory. Are you thinking of some other pattern that would leak memory? If so, what is it? I can only think of patterns where a logic error is also involved. That is, where the meaningless value of 'val' is used in code paths where err != nil. func alwaysError() (*int, error) { var dummy int // We return &dummy even though it's a meaningless value. // Will this cause a memory leak? return &dummy, fmt.Errorf("An error") } func caller() { val, err := alwaysError() if err != nil { fmt.Printf("Error\n") // It won't! Because the value pointed to by 'val' // can be GCed from this point on. return } // never get here fmt.Printf("Value %v\n", val) // // ...lots of code that uses *val... // } You might think that you'd get a leak if the error branch was also long-lived, but Go's GC seems to be precise with respect to conditional branching (as far as I can tell by experimenting with runtime.SetFinalizer). That is, as long as the error branch doesn't refer to 'val', then the value pointed to by 'val' can be collected once the error branch is entered (and before 'caller' returns).
    - amluto 4y ago
      I haven't actually played with this, but if the same pattern of lazily (sloppily?) returning arbitrary pointers along with errors continues, the lifetime ought to be extended arbitrarily. The caller could itself return val, err, and so on up the chain.
      - foldr 4y ago
        I'm not seeing how that would cause a memory leak. It just means that the value might be GCed a bit later. Could you give an example?
        amluto 4y ago
        It’s a temporary leak. My point is that it’s an actual side effect, unlike how in C it has no effect whatsoever if the value is ignored. In a language where destruction of an object often releases external resources (C++, for example, or Python or Rust to a lesser extent), this effect would be more dramatic. Imagine someone doing this in Python: def func(): f = tmpfile(…) err = do something return f, err This style would be absurd and wrong — it keeps f alive to long.

za3faran 4y ago

What I found frustrating that there is nothing stopping a function from returning both a non nil value and a non nil error. I've seen weird code that did that and it could have easily resulted in incorrect behavior.

klodolph 4y ago
The one place where you’ll actually see this is in Read(). If you try to read 1000 bytes, you might get “800 bytes read, EOF” as a result. The worst part is that os.File won’t ever do this!
- ok_dad 4y ago
  I really hate when people use "errors" for signals. Or, alternately, use the signals mechanisms for actual errors. An error should be "something went wrong", and reading a file to the EOF is not "going wrong", that's just what the "read()" should do if you tell it so! I like Go's multiple returns and error checking by default, but it definitely should have been implemented with some sort of "Result/Error" type union instead. Go made so many good decisions, that I am frankly amazed they made this one really bad one.
  - robotresearcher 4y ago
    If you take the position that the semantics of read() are 'read data from this endless stream', then EOF is an error indicating that this model no longer applies. Doesn't bother me at all.
    - account42 4y ago
      Except in that model it would be fine to return an error as soon as soon as you know there is an EOF while in the real world you do want to read right up to the EOF because EOF is usually not an error condition but expected, even with streams of indeterminate length.
      - 4y ago