> On Feb. 2, 2017, 11:46 a.m., Alex Clemmer wrote: > > 3rdparty/libprocess/src/io.cpp, line 222 > > <https://reviews.apache.org/r/54602/diff/5/?file=1600504#file1600504line222> > > > > This comparison is definitely a bug. On Windows, when this `fd` is a > > pipe `HANDLE`, this comparison fails even for valid values of `fd`. > > > > This can, among other things, cause the Windows Executor to hang > > indefinitely in some scenarios. For example, when we call `docker inspect`, > > we will probably end up writing more data to the pipe than the pipe has > > buffer, so the Executor will block until we `io::read` it. But since this > > comparison fails, we will never complete the read. Hence, the Executor > > hangs indefinitely. > > > > This comparison can be made to work on Windows by making it properly > > check whether the `HANDLE` (using, _e.g._, `fd == > > os::WindowsFd(INVALID_HANDLE_VALUE)`) is invalid, but this obviously won't > > work on POSIX. In general, I suspect these comparisons are fraught, so I > > think we should consider removing them and having utility functions such as > > `fd.valid()` or something.
We explicitly support these comparisons, and they're handled by the complex logic in `operator<` for `WindowsFD`. There was a bug that you were running into I believe, which I have addressed. Thanks! - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54602/#review164008 ----------------------------------------------------------- On Feb. 4, 2017, 5:39 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54602/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2017, 5:39 p.m.) > > > Review request for mesos, Daniel Pravat and Joris Van Remoortere. > > > Repository: mesos > > > Description > ------- > > Replaced `int` with `int_fd` in libprocess. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/io.hpp > b342333bc7f2ae12b5b6a92fa21896c8f42353cb > 3rdparty/libprocess/include/process/network.hpp > 8234765e23bb3d434da0b0f818fd42569d554ab8 > 3rdparty/libprocess/include/process/posix/subprocess.hpp > a1054e788ef6a322901c262380aceab8192235ac > 3rdparty/libprocess/include/process/socket.hpp > 5e958addc6012d85d1dd9025d27b258265807a9e > 3rdparty/libprocess/include/process/subprocess_base.hpp > 1c9f2e33a80e457857985d9b5663c8cb2a089505 > 3rdparty/libprocess/include/process/windows/subprocess.hpp > f7beba16d6ed56be45bf5f6221e89a5b0cbcf1e7 > 3rdparty/libprocess/src/encoder.hpp > 1447d6f93c15b9f3d134507ecb0bda020a218a49 > 3rdparty/libprocess/src/http.cpp f12a8a5d71f09c7413dae3e6192706ae63972923 > 3rdparty/libprocess/src/io.cpp e31f176a4ef110e9c7d0e5efd88c610e30718d2f > 3rdparty/libprocess/src/libev_poll.cpp > da2a78bd8aa4ed8c37bd2ca16d148b73112aa0e7 > 3rdparty/libprocess/src/libevent_poll.cpp > 0803ba33622c86df38b3efd4f1e3197edf93a0af > 3rdparty/libprocess/src/libevent_ssl_socket.hpp > 97ce339c8183b4a4ffc4e37f052c9d2adb79654d > 3rdparty/libprocess/src/libevent_ssl_socket.cpp > fec131f912174e2f9c1b1060a45158d6a61c43c5 > 3rdparty/libprocess/src/poll_socket.hpp > 89789e6bb91d79e4de1c4f4be3882df851845930 > 3rdparty/libprocess/src/poll_socket.cpp > 93ca37f105527fb225574107480114ee5ac74c76 > 3rdparty/libprocess/src/process.cpp > d1331238f6c9df47660bae8cdaa5e8c8add70212 > 3rdparty/libprocess/src/socket.cpp b819503095261c77f42d6f20d1a4b2b6170fb4e1 > 3rdparty/libprocess/src/subprocess.cpp > b89dc7ebb86604fad7ca14a9750ef3faff7efbed > 3rdparty/libprocess/src/subprocess_posix.cpp > 19271414f145d23f50ac07570c48782819f382b4 > 3rdparty/libprocess/src/subprocess_windows.cpp > 20cad52d4a4d7fc51487e150a849972eb19ed08e > 3rdparty/libprocess/src/tests/io_tests.cpp > 5c889e97cb1402a98b27cad2f3dbb4047a995506 > 3rdparty/libprocess/src/tests/ssl_tests.cpp > a32d20e47f67d88bbe5928e0ddc129745c5f42e0 > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > 59c17692012ddfb540ecdd48560c73c42a15f061 > > Diff: https://reviews.apache.org/r/54602/diff/ > > > Testing > ------- > > > Thanks, > > Michael Park > >
