You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2018/02/06 00:27:57 UTC
[1/2] mesos git commit: Fixed SSL socket shutdown returned errno.
Repository: mesos
Updated Branches:
refs/heads/master d831e27b7 -> a79fe26ac
Fixed SSL socket shutdown returned errno.
Review: https://reviews.apache.org/r/65475/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a79fe26a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a79fe26a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a79fe26a
Branch: refs/heads/master
Commit: a79fe26ac84854f44e8d6a7a9bf0e9eba55bafef
Parents: 900ad67
Author: Till Toenshoff <to...@me.com>
Authored: Mon Feb 5 23:53:54 2018 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Tue Feb 6 01:26:59 2018 +0100
----------------------------------------------------------------------
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a79fe26a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 0ceebda..4de161d 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -165,7 +165,13 @@ Try<Nothing, SocketError> LibeventSSLSocketImpl::shutdown(int how)
CHECK(recv_request.get() == nullptr);
CHECK(send_request.get() == nullptr);
- return SocketError(ENOTCONN);
+ // We expect this to fail and generate an 'ENOTCONN' failure as
+ // no connection should exist at this point.
+ if (::shutdown(s, how) < 0) {
+ return SocketError();
+ }
+
+ return Nothing();
}
}
[2/2] mesos git commit: Updated socket shutdown to return SocketError.
Posted by ti...@apache.org.
Updated socket shutdown to return SocketError.
Socket::shutdown returns SocketError allowing for higher level
functions to have more control over logging errors where needed.
Also switches SocketManager::close's shutdown failure logging
back to ERROR level as we are now able to filter out expected
failures.
Review: https://reviews.apache.org/r/65474/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/900ad678
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/900ad678
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/900ad678
Branch: refs/heads/master
Commit: 900ad678138b2cd3deb5869604c8f25b6bb29f8c
Parents: d831e27
Author: Till Toenshoff <to...@me.com>
Authored: Mon Feb 5 23:53:37 2018 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Tue Feb 6 01:26:59 2018 +0100
----------------------------------------------------------------------
3rdparty/libprocess/include/process/socket.hpp | 7 +++--
3rdparty/libprocess/src/http.cpp | 6 ++--
3rdparty/libprocess/src/libevent_ssl_socket.cpp | 4 +--
3rdparty/libprocess/src/libevent_ssl_socket.hpp | 2 +-
3rdparty/libprocess/src/process.cpp | 32 +++++++++++++-------
3rdparty/libprocess/src/tests/ssl_tests.cpp | 4 ++-
6 files changed, 35 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/include/process/socket.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/socket.hpp b/3rdparty/libprocess/include/process/socket.hpp
index ae6154d..4f0f6e9 100644
--- a/3rdparty/libprocess/include/process/socket.hpp
+++ b/3rdparty/libprocess/include/process/socket.hpp
@@ -24,6 +24,7 @@
#include <process/future.hpp>
#include <stout/abort.hpp>
+#include <stout/error.hpp>
#include <stout/nothing.hpp>
#include <stout/try.hpp>
#include <stout/unreachable.hpp>
@@ -187,10 +188,10 @@ public:
* Shuts down the socket. Accepts an integer which specifies the
* shutdown mode.
*/
- virtual Try<Nothing> shutdown(int how)
+ virtual Try<Nothing, SocketError> shutdown(int how)
{
if (::shutdown(s, how) < 0) {
- return ErrnoError();
+ return SocketError();
}
return Nothing();
@@ -396,7 +397,7 @@ public:
// TODO(benh): Replace the default to Shutdown::READ_WRITE or remove
// all together since it's unclear what the default should be.
- Try<Nothing> shutdown(Shutdown shutdown = Shutdown::READ)
+ Try<Nothing, SocketError> shutdown(Shutdown shutdown = Shutdown::READ)
{
int how = [&]() {
switch (shutdown) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/src/http.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp
index cc41fa6..76a65e0 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -46,6 +46,7 @@
#include <process/socket.hpp>
#include <process/state_machine.hpp>
+#include <stout/error.hpp>
#include <stout/foreach.hpp>
#include <stout/ip.hpp>
#include <stout/lambda.hpp>
@@ -1152,7 +1153,7 @@ public:
Future<Nothing> disconnect(const Option<string>& message = None())
{
- Try<Nothing> shutdown = socket.shutdown(
+ Try<Nothing, SocketError> shutdown = socket.shutdown(
network::Socket::Shutdown::READ_WRITE);
// If a response is still streaming, we send EOF to
@@ -1170,7 +1171,8 @@ public:
disconnection.set(Nothing());
- return shutdown;
+ return shutdown.isSome() ? Future<Nothing>(Nothing())
+ : Failure(shutdown.error().message);
}
Future<Nothing> disconnected()
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/src/libevent_ssl_socket.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.cpp b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
index 521b0cf..0ceebda 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.cpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.cpp
@@ -154,7 +154,7 @@ void LibeventSSLSocketImpl::initialize()
}
-Try<Nothing> LibeventSSLSocketImpl::shutdown(int how)
+Try<Nothing, SocketError> LibeventSSLSocketImpl::shutdown(int how)
{
// Nothing to do if this socket was never initialized.
synchronized (lock) {
@@ -165,7 +165,7 @@ Try<Nothing> LibeventSSLSocketImpl::shutdown(int how)
CHECK(recv_request.get() == nullptr);
CHECK(send_request.get() == nullptr);
- return ErrnoError(ENOTCONN);
+ return SocketError(ENOTCONN);
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/src/libevent_ssl_socket.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/libevent_ssl_socket.hpp b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
index 640fa67..6ef5a86 100644
--- a/3rdparty/libprocess/src/libevent_ssl_socket.hpp
+++ b/3rdparty/libprocess/src/libevent_ssl_socket.hpp
@@ -56,7 +56,7 @@ public:
// do not have a concept of read/write-only shutdown. If either end
// of the socket is closed, then the futures of any outstanding read
// requests will be completed (possibly as failures).
- Try<Nothing> shutdown(int how) override;
+ Try<Nothing, SocketError> shutdown(int how) override;
// We need a post-initializer because 'shared_from_this()' is not
// valid until the constructor has finished.
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index ba9bc29..42e7adf 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -94,6 +94,7 @@
#endif // __WINDOWS__
#include <stout/duration.hpp>
+#include <stout/error.hpp>
#include <stout/flags.hpp>
#include <stout/foreach.hpp>
#include <stout/lambda.hpp>
@@ -1635,9 +1636,10 @@ void SocketManager::link(
// the final socket reference. This will not result in an
// `ExitedEvent` because we have already removed the `existing`
// socket from the mapping of linkees and linkers.
- Try<Nothing> shutdown = existing.shutdown();
+ Try<Nothing, SocketError> shutdown = existing.shutdown();
if (shutdown.isError()) {
- VLOG(1) << "Failed to shutdown old link: " << shutdown.error();
+ VLOG(1) << "Failed to shutdown old link: "
+ << shutdown.error().message;
}
connect = true;
@@ -2081,7 +2083,7 @@ Encoder* SocketManager::next(int_fd s)
Socket socket = iterator->second;
sockets.erase(iterator);
- Try<Nothing> shutdown = socket.shutdown();
+ Try<Nothing, SocketError> shutdown = socket.shutdown();
// Failure here could be due to reasons including that the underlying
// socket is already closed so it by itself doesn't necessarily
@@ -2091,7 +2093,7 @@ Encoder* SocketManager::next(int_fd s)
<< ", address " << (socket.address().isSome()
? stringify(socket.address().get())
: "N/A")
- << ": " << shutdown.error();
+ << ": " << shutdown.error().message;
}
}
}
@@ -2173,13 +2175,21 @@ void SocketManager::close(int_fd s)
// Failure here could be due to reasons including that the underlying
// socket is already closed so it by itself doesn't necessarily
// suggest anything wrong.
- Try<Nothing> shutdown = socket.shutdown();
- if (shutdown.isError()) {
- LOG(INFO) << "Failed to shutdown socket with fd " << socket.get()
- << ", address " << (socket.address().isSome()
- ? stringify(socket.address().get())
- : "N/A")
- << ": " << shutdown.error();
+ Try<Nothing, SocketError> shutdown = socket.shutdown();
+
+ // Avoid logging an error when the shutdown was triggered on a
+ // socket that is not connected.
+ if (shutdown.isError() &&
+#ifdef __WINDOWS__
+ shutdown.error().code != WSAENOTCONN) {
+#else // __WINDOWS__
+ shutdown.error().code != ENOTCONN) {
+#endif // __WINDOWS__
+ LOG(ERROR) << "Failed to shutdown socket with fd " << socket.get()
+ << ", address " << (socket.address().isSome()
+ ? stringify(socket.address().get())
+ : "N/A")
+ << ": " << shutdown.error().message;
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/900ad678/3rdparty/libprocess/src/tests/ssl_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/ssl_tests.cpp b/3rdparty/libprocess/src/tests/ssl_tests.cpp
index b1a0ea6..71f2df8 100644
--- a/3rdparty/libprocess/src/tests/ssl_tests.cpp
+++ b/3rdparty/libprocess/src/tests/ssl_tests.cpp
@@ -916,7 +916,9 @@ TEST_F(SSLTest, ShutdownThenSend)
AWAIT_ASSERT_READY(socket);
- EXPECT_SOME(Socket(socket.get()).shutdown());
+ // TODO(tillt): Workaround for the lack of EXPECT_SOME for
+ // typed errors in a `Try`. See MESOS-7220.
+ EXPECT_TRUE(Socket(socket.get()).shutdown().isSome());
// This send should fail now that the socket is shut down.
AWAIT_FAILED(Socket(socket.get()).send("Hello World"));