nilaway icon indicating copy to clipboard operation
nilaway copied to clipboard

Rethink our strategies towards handling channels

Open yuxincs opened this issue 5 months ago • 0 comments

Currently in NilAway we forbid sending to and receiving from nilable channels by creating corresponding consumers for them. However, in practice this will not result in panics.

See the following behaviors regarding nil / closed channels (taken from [this SO post], slightly modified (https://stackoverflow.com/questions/43616434/closed-channel-vs-nil-channel)):

  1. A send to a nil channel blocks forever
  2. A receive from a nil channel blocks forever
  3. A send to a closed channel panics
  4. A receive from a closed channel returns the zero value immediately

Only case 3 (sending to a closed channel) will result in a panic. The other cases arguably may not be within scope of NilAway's analysis. Moreover, NilAway currently only tracks nilabilities of channels, but not the states of them, making it unable to really handle case 2 and 4.

We will definitely need to rethink our strategies towards handling channels since the current approach raises a lot of false positives. For example, below is a perfectly valid and very popular pattern in Go:

func foo(ctx context.Context) {
  // ...
  select {
    case ctx.Done():
      return
    default:
  }
  // ...
}

which checks if the context is done, and if not continue to handling next rounds of business. Note that here ctx.Done() return a nilable channel, but it is perfectly fine to read from it (without panics, and in this case, even without blocking forever due to the select statement).

See also issue #98 for more discussions.

yuxincs avatar Feb 01 '24 02:02 yuxincs