tree f51af9a380ba3ea40f6bceea436ebde694694662
parent 97331e3cda6a7f17f3aa3562b04d1a65ec64ba68
author Asim Shankar <ashankar@google.com> 1461954855 -0400
committer Asim Shankar <ashankar@google.com> 1461954855 -0400

netconfig/roaming: Fix bug and simplify.

This patch began as a fix to a bug in handling of the socket used to
monitoring network configuration changes. Without this patch,
it is possible for one thread to be in a loop that repeatedly
reads from a file descriptor (recvfrom() in rtnetlinkWatcher.watcher in
Linux, read() in bsdNetConfigWatcher.watcher in BSD) while another
thread concurrently closes the same file descriptor
(NetconfigWatcher.Stop implementation in both).

This is fraught with peril. Quoting the 'manpage' of 'close':
"It  is  probably  unwise to close file descriptors while they may be in use by
system calls in other threads in the same process.  Since a file descriptor may
be reused, there are some obscure race conditions that may cause unintended
side effects."

Indeed, these side effects were causing flaky tests in some unrelated changes I
was working on.

In order to fix this, there were some options:
- Call "close" only from the goroutine executing "recvfrom", and use
  "shutdown" in Stop() to unblock any threads stuck in the kernel waiting
  on "recvfrom". Unfortunately, it seems that AF_NETLINK sockets in linux
  do not support "shutdown"
- Use some other scheme (such as poll/epoll/select on Linux and poll/kqueue on
  BSD) to be able to close the file descriptor in the same goroutine that
  executes reads. This was a lot of OS-specific code, which seemed
  to be more complicated than it was worth (see
  https://vanadium-review.googlesource.com/#/c/21811/ for example), OR
- Don't bother about doing a clean shutdown at all: If a process starts
  roaming network servers, then live with one file descriptor that will
  never be closed and one goroutine that might never exit.

This patch takes that last approach. In the process, layers of abstraction
are reduced and overall the code becomes tighter. For example, instead of
netconfig.NetConfigWatcher --> pubub.Publisher --> roaming.ReadRoamingStream
--> flow/manager.{add,rm}Addrs,

we have:

netconfig.NotifyChange --> flow/manager.updateAddresses

MultiPart: 1/2
Change-Id: I9182927025ab8879b7fd22fdd4bb2979ae5c2aa7