avoid race in Lwt.pick

avoid race in Lwt.pick

Summary:
There's a race in clientIdeService.serve...
```
Lwt.pick [
  send_queued_up_messages t.out_fd ...;
  emit_messages_from_daemon t.in_fd ...;
]
```

Both forks of the pick involve `marshal_tools_lwt`. In other words, they each involve a sequence of async writes, or a sequence of async reads.

Imagine if both forks start, and both perform part of their sequence of reads, but then one of the forks wins. Lwt will "attempt to cancel" the other fork.
* Imagine if it cancels the write fork after only some of the async writes -- the next time we write a message, then what the consumer unmarshals will be a weird amalgam of some bytes from the start of one message and some bytes from the start of the next, which will lead to a marshal error.
* Imagine if it cancels the read fork after only some of the async reads -- the next time we read a message, what we unmarshal will be an amalgam of some bytes from the end of one message and some bytes from the start of the next, which will lead to a marshal error.

We have been observing marshal errors in the wild. This seems the likeliest explannation.

WHAT THIS DIFF DOES:

I rewrote the "serve" loop to avoid the race. I copied the style from ClientLsp.get_message_source, which also uses `Lwt_unix.wait_read` to discover whether there's an incoming message available. The call to `wait_read` is easily cancellable with no harm done.

Another option would have been to make Marshal_tools atomic. We still probably should do that. But I picked the avenue in this diff because it was straightforward, and because I think it makes the code look cleaner. I think that multiple async loops, while possibile, make the code harder to reason about.

I took the opportunity to factor out the "shutdown" action and document it a bit better.

Differential Revision: D18927910

fbshipit-source-id: 8a8694eb95ba221e084cd117e8cf5debe09c2107