From 1cd1e0ebee2e43dd6c87464db649f91616f38b34 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 17:13:59 -0400 Subject: [PATCH 01/11] Refactor dispatch pipeline with the addition of a public dispatch(request, sendResponse) on Object. --- .vscode/settings.json | 4 +- cpp/include/Ice/AsyncResponseHandler.h | 87 +++ cpp/include/Ice/Incoming.h | 121 ---- cpp/include/Ice/IncomingRequest.h | 71 +++ cpp/include/Ice/LocalException.h | 39 -- cpp/include/Ice/MarshaledResult.h | 13 +- cpp/include/Ice/Object.h | 49 +- cpp/include/Ice/ObjectAdapter.h | 7 + cpp/include/Ice/OutgoingResponse.h | 166 ++++++ cpp/include/Ice/ResponseHandlerF.h | 17 - cpp/src/Ice/CollocatedRequestHandler.cpp | 54 +- cpp/src/Ice/CollocatedRequestHandler.h | 19 +- cpp/src/Ice/ConnectionI.cpp | 103 ++-- cpp/src/Ice/ConnectionI.h | 29 +- cpp/src/Ice/Exception.cpp | 7 - cpp/src/Ice/Incoming.cpp | 672 ----------------------- cpp/src/Ice/IncomingRequest.cpp | 66 +++ cpp/src/Ice/LocalException.cpp | 9 - cpp/src/Ice/LoggerMiddleware.cpp | 119 ++++ cpp/src/Ice/LoggerMiddleware.h | 35 ++ cpp/src/Ice/MarshaledResult.cpp | 10 +- cpp/src/Ice/Object.cpp | 175 +++--- cpp/src/Ice/ObjectAdapterI.cpp | 35 +- cpp/src/Ice/ObjectAdapterI.h | 5 +- cpp/src/Ice/ObserverMiddleware.cpp | 97 ++++ cpp/src/Ice/ObserverMiddleware.h | 27 + cpp/src/Ice/OutgoingResponse.cpp | 314 +++++++++++ cpp/src/Ice/ResponseHandler.h | 29 - cpp/src/Ice/ServantManager.cpp | 78 +++ cpp/src/Ice/ServantManager.h | 14 +- cpp/src/slice2cpp/Gen.cpp | 116 ++-- 31 files changed, 1420 insertions(+), 1167 deletions(-) create mode 100644 cpp/include/Ice/AsyncResponseHandler.h delete mode 100644 cpp/include/Ice/Incoming.h create mode 100644 cpp/include/Ice/IncomingRequest.h create mode 100644 cpp/include/Ice/OutgoingResponse.h delete mode 100644 cpp/include/Ice/ResponseHandlerF.h delete mode 100644 cpp/src/Ice/Incoming.cpp create mode 100644 cpp/src/Ice/IncomingRequest.cpp create mode 100644 cpp/src/Ice/LoggerMiddleware.cpp create mode 100644 cpp/src/Ice/LoggerMiddleware.h create mode 100644 cpp/src/Ice/ObserverMiddleware.cpp create mode 100644 cpp/src/Ice/ObserverMiddleware.h create mode 100644 cpp/src/Ice/OutgoingResponse.cpp delete mode 100644 cpp/src/Ice/ResponseHandler.h diff --git a/.vscode/settings.json b/.vscode/settings.json index 3d4a9371dd7..f76f8509461 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -73,8 +73,8 @@ "unordered_map": "cpp", "variant": "cpp", "algorithm": "cpp", - "xtree": "cpp" + "xtree": "cpp", + "execution": "cpp" }, "C_Cpp.default.cppStandard": "c++20", - "editor.formatOnSave": true } diff --git a/cpp/include/Ice/AsyncResponseHandler.h b/cpp/include/Ice/AsyncResponseHandler.h new file mode 100644 index 00000000000..d392d72ea61 --- /dev/null +++ b/cpp/include/Ice/AsyncResponseHandler.h @@ -0,0 +1,87 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#ifndef ICE_ASYNC_RESPONSE_HANDLER_H +#define ICE_ASYNC_RESPONSE_HANDLER_H + +#include "Current.h" +#include "OutgoingResponse.h" +#include "LocalException.h" + +#include + +namespace IceInternal +{ + // This class helps with the implementation of the AMD response and exception callbacks. It allows the dispatch + // thread and these two callbacks to share the same sendResponse and Current objects, and ensures sendResponse is + // called exactly once. + class AsyncResponseHandler final + { + public: + // This class typically holds a _copy_ of the incoming request current object. + AsyncResponseHandler(std::function sendResponse, Ice::Current current) + : _sendResponse(std::move(sendResponse)), + _current(std::move(current)) + { + } + + void sendEmptyResponse() noexcept + { + if (!_responseSent.test_and_set()) + { + _sendResponse(makeEmptyOutgoingResponse(_current)); + } + // else we ignore this call. + } + + void sendResponse(Ice::MarshaledResult marshaledResult) noexcept + { + if (!_responseSent.test_and_set()) + { + _sendResponse(Ice::OutgoingResponse{marshaledResult.outputStream(), _current}); + } + // else we ignore this call. + } + + void sendResponse(bool ok, const std::pair& encaps) noexcept + { + if (!_responseSent.test_and_set()) + { + _sendResponse(makeOutgoingResponse(ok, encaps, _current)); + } + // else we ignore this call. + } + + void sendResponse( + std::function marshal, + Ice::FormatType format = Ice::FormatType::DefaultFormat) noexcept + { + // It is critical to only call the _sendResponse function only once. Calling it multiple times results in an + // incorrect dispatch count. + if (!_responseSent.test_and_set()) + { + _sendResponse(makeOutgoingResponse(std::move(marshal), _current, format)); + } + // else we ignore this call. + } + + void sendException(std::exception_ptr ex) noexcept + { + if (!_responseSent.test_and_set()) + { + _sendResponse(makeOutgoingResponse(ex, _current)); + } + // else we ignore this call. + } + + const Ice::Current& current() const noexcept { return _current; } + + private: + const std::function _sendResponse; + const Ice::Current _current; + std::atomic_flag _responseSent = ATOMIC_FLAG_INIT; + }; +} + +#endif diff --git a/cpp/include/Ice/Incoming.h b/cpp/include/Ice/Incoming.h deleted file mode 100644 index 3414be1dbb0..00000000000 --- a/cpp/include/Ice/Incoming.h +++ /dev/null @@ -1,121 +0,0 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// - -#ifndef ICE_INCOMING_H -#define ICE_INCOMING_H - -#include "Current.h" -#include "InputStream.h" -#include "InstanceF.h" -#include "MarshaledResult.h" -#include "Object.h" -#include "ObserverHelper.h" -#include "OutputStream.h" -#include "ResponseHandlerF.h" -#include "ServantManagerF.h" - -namespace Ice -{ - class ServantLocator; -} - -namespace IceInternal -{ - class ICE_API Incoming final - { - public: - Incoming( - Instance*, - ResponseHandlerPtr, - Ice::ConnectionPtr, - Ice::ObjectAdapterPtr, - bool, - std::uint8_t, - std::int32_t); - Incoming(Incoming&&); - Incoming(const Incoming&) = delete; - Incoming& operator=(const Incoming&) = delete; - - Ice::OutputStream* startWriteParams(); - void endWriteParams(); - void writeEmptyParams(); - void writeParamEncaps(const std::uint8_t*, std::int32_t, bool ok); - void setMarshaledResult(Ice::MarshaledResult&&); - - void setFormat(Ice::FormatType format) { _format = format; } - - void invoke(const ServantManagerPtr&, Ice::InputStream*); - - // Inlined for speed optimization. - void skipReadParams() { _current.encoding = _is->skipEncapsulation(); } - Ice::InputStream* startReadParams() - { - // - // Remember the encoding used by the input parameters, we'll - // encode the response parameters with the same encoding. - // - _current.encoding = _is->startEncapsulation(); - return _is; - } - void endReadParams() const { _is->endEncapsulation(); } - void readEmptyParams() { _current.encoding = _is->skipEmptyEncapsulation(); } - void readParamEncaps(const std::uint8_t*& v, std::int32_t& sz) - { - _current.encoding = _is->readEncapsulation(v, sz); - } - - const Ice::Current& current() const { return _current; } - - // Async dispatch writes an empty response and completes successfully. - void response(); - - // Async dispatch writes a marshaled result and completes successfully. - void response(Ice::MarshaledResult&& marshaledResult); - - // Async dispatch completes successfully. Call this function after writing the response. - void completed(); - - // Async dispatch completes with an exception. This can throw, for example, if the application already called - // response(), completed() or failed(). - void completed(std::exception_ptr ex); - - // Handle an exception that was thrown by an async dispatch. Use this function from the dispatch thread. - void failed(std::exception_ptr) noexcept; - - private: - void setResponseSent(); - - void sendResponse(); - void sendException(std::exception_ptr); - - void warning(const Ice::Exception&) const; - void warning(std::exception_ptr) const; - - bool servantLocatorFinished(); - - void handleException(std::exception_ptr); - - Ice::Current _current; - std::shared_ptr _servant; - std::shared_ptr _locator; - std::shared_ptr _cookie; - DispatchObserver _observer; - bool _isTwoWay; - std::uint8_t _compress; - Ice::FormatType _format; - Ice::OutputStream _os; - - ResponseHandlerPtr _responseHandler; - - // _is points to an object allocated on the stack of the dispatch thread. - Ice::InputStream* _is; - - // This flag is set when the user calls an async response or exception callback. A second call is incorrect and - // results in ResponseSentException. - // We don't need an atomic flag since it's purely to detect logic errors in the application code. - bool _responseSent = false; - }; -} - -#endif diff --git a/cpp/include/Ice/IncomingRequest.h b/cpp/include/Ice/IncomingRequest.h new file mode 100644 index 00000000000..b426eb9e462 --- /dev/null +++ b/cpp/include/Ice/IncomingRequest.h @@ -0,0 +1,71 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#ifndef ICE_INCOMING_REQUEST_H +#define ICE_INCOMING_REQUEST_H + +#include "ConnectionF.h" +#include "Current.h" +#include "ObjectAdapterF.h" + +namespace Ice +{ + class InputStream; + + /** + * Represent a request received by a connection. It's the argument to the dispatch function on Object. + * @remarks IncomingRequest is neither copyable nor movable. It can be used only on the dispatch thread. + * @see Object::dispatch + * \headerfile Ice/Ice.h + */ + class ICE_API IncomingRequest final + { + public: + /** + * Construct an IncomingRequest object. + * @param requestId The request ID. It's 0 for oneway requests. + * @param connection The connection that received the request. It's null for collocated invocations. + * @param adapter The object adapter to set in Current. + * @param inputStream The input stream buffer over the incoming Ice protocol request message. The stream is + * positioned at the beginning of the request header - the next data to read is the identity of the target. + */ + IncomingRequest( + int32_t requestId, + ConnectionPtr connection, + ObjectAdapterPtr adapter, + InputStream& inputStream); + + IncomingRequest(const IncomingRequest&) = delete; + IncomingRequest(IncomingRequest&&) noexcept = delete; + IncomingRequest& operator=(const IncomingRequest&) = delete; + IncomingRequest& operator=(IncomingRequest&&) = delete; + + /** + * Return the current object of the request. + */ + Current& current() noexcept { return _current; } + + /** + * Return the current object of the request. + */ + const Ice::Current& current() const noexcept { return _current; } + + /** + * Return the input stream buffer of the request. + */ + InputStream& inputStream() noexcept { return _inputStream; } + + /** + * Return the number of bytes in the request. These are all the bytes starting with the identity of the target. + */ + std::int32_t size() const { return _requestSize; } + + private: + InputStream& _inputStream; + Current _current; + std::int32_t _requestSize; + }; +} + +#endif diff --git a/cpp/include/Ice/LocalException.h b/cpp/include/Ice/LocalException.h index 16c350f8909..f4740c82aa5 100644 --- a/cpp/include/Ice/LocalException.h +++ b/cpp/include/Ice/LocalException.h @@ -3569,45 +3569,6 @@ namespace Ice */ ICE_MEMBER(ICE_API) virtual void ice_print(::std::ostream& stream) const override; }; - - /** - * Indicates the application attempted to call the callbacks for an async dispatch more than once, or called an - * async dispatch callback after throwing an exception from the dispatch thread. \headerfile Ice/Ice.h - */ - class ICE_CLASS(ICE_API) ResponseSentException : public LocalExceptionHelper - { - public: - ICE_MEMBER(ICE_API) virtual ~ResponseSentException(); - - ResponseSentException(const ResponseSentException&) = default; - - /** - * The file and line number are required for all local exceptions. - * @param file The file name in which the exception was raised, typically __FILE__. - * @param line The line number at which the exception was raised, typically __LINE__. - */ - ResponseSentException(const char* file, int line) - : LocalExceptionHelper(file, line) - { - } - - /** - * Obtains a tuple containing all of the exception's data members. - * @return The data members in a tuple. - */ - std::tuple<> ice_tuple() const { return std::tie(); } - - /** - * Obtains the Slice type ID of this exception. - * @return The fully-scoped type ID. - */ - ICE_MEMBER(ICE_API) static ::std::string_view ice_staticId(); - /** - * Prints this exception to the given stream. - * @param stream The target stream. - */ - ICE_MEMBER(ICE_API) virtual void ice_print(::std::ostream& stream) const override; - }; } /// \cond STREAM diff --git a/cpp/include/Ice/MarshaledResult.h b/cpp/include/Ice/MarshaledResult.h index e3e6a16e416..976db8725fa 100644 --- a/cpp/include/Ice/MarshaledResult.h +++ b/cpp/include/Ice/MarshaledResult.h @@ -32,9 +32,11 @@ namespace Ice MarshaledResult& operator=(const MarshaledResult&) = delete; MarshaledResult& operator=(MarshaledResult&&); - protected: /// \cond INTERNAL + OutputStream& outputStream() noexcept { return _ostr; } + + protected: /** * The constructor requires the Current object that was passed to the servant. */ @@ -43,15 +45,6 @@ namespace Ice /** The output stream used to marshal the results. */ OutputStream _ostr; - private: - friend class IceInternal::Incoming; - - /** - * Swaps the output stream of this object with the supplied output stream. - * @param other The output stream to swap with. - */ - void swap(OutputStream& other); - /// \endcond }; } diff --git a/cpp/include/Ice/Object.h b/cpp/include/Ice/Object.h index fc5a72b87d8..05991b9a38d 100644 --- a/cpp/include/Ice/Object.h +++ b/cpp/include/Ice/Object.h @@ -10,6 +10,8 @@ #include #include #include +#include "IncomingRequest.h" +#include "OutgoingResponse.h" namespace IceInternal { @@ -31,6 +33,24 @@ namespace Ice public: virtual ~Object() = default; + /** + * Dispatch an incoming request and return the corresponding outgoing response. + * @param request The incoming request. + * @param sendResponse A callback that the implementation calls to return the response. sendResponse does not + * throw any exception and any sendResponse wrapper must not throw any exception. sendResponse can be called by + * the thread that called dispatch (the "dispatch thread") or by another thread. The implementation must call + * sendResponse exactly once or throw an exception. + * @remarks Calling sendResponse can be thought as returning the outgoing response. Just like when you return a + * value from a remote operation, you can only return it once and you don't know if the client receives this + * value. In practice, the Ice-provided sendResponse attempts to send the response to the client synchronously, + * but may send it asynchronously. It can also silently fail to send the response back to the client. + * This function is the main building block for the Ice dispatch pipeline. The implementation provided by the + * base Object class dispatches incoming requests to the four Object operations (ice_isA, ice_ping, ice_ids and + * ice_id), and throws OperationNotExistException for all other operations. This base implementation is trivial + * and should be overridden and fully replaced by all derived classes. + */ + virtual void dispatch(IncomingRequest& request, std::function sendResponse); + /** * Tests whether this object supports a specific Slice interface. * @param s The type ID of the Slice interface to test against. @@ -41,7 +61,7 @@ namespace Ice */ virtual bool ice_isA(std::string s, const Current& current) const; /// \cond INTERNAL - bool _iceD_ice_isA(IceInternal::Incoming&); + void _iceD_ice_isA(IncomingRequest&, std::function); /// \endcond /** @@ -50,7 +70,7 @@ namespace Ice */ virtual void ice_ping(const Current& current) const; /// \cond INTERNAL - bool _iceD_ice_ping(IceInternal::Incoming&); + void _iceD_ice_ping(IncomingRequest&, std::function); /// \endcond /** @@ -60,7 +80,7 @@ namespace Ice */ virtual std::vector ice_ids(const Current& current) const; /// \cond INTERNAL - bool _iceD_ice_ids(IceInternal::Incoming&); + void _iceD_ice_ids(IncomingRequest&, std::function); /// \endcond /** @@ -70,7 +90,7 @@ namespace Ice */ virtual std::string ice_id(const Current& current) const; /// \cond INTERNAL - bool _iceD_ice_id(IceInternal::Incoming&); + void _iceD_ice_id(IncomingRequest&, std::function); /// \endcond /** @@ -79,10 +99,6 @@ namespace Ice */ static std::string_view ice_staticId(); - /// \cond INTERNAL - virtual bool _iceDispatch(IceInternal::Incoming&); - /// \endcond - protected: /// \cond INTERNAL static void _iceCheckMode(OperationMode, OperationMode); @@ -114,9 +130,7 @@ namespace Ice std::vector& outEncaps, const Current& current) = 0; - /// \cond INTERNAL - bool _iceDispatch(IceInternal::Incoming&) final; - /// \endcond + void dispatch(IncomingRequest&, std::function) final; }; /** @@ -144,9 +158,7 @@ namespace Ice std::vector& outEncaps, const Current& current) = 0; - /// \cond INTERNAL - bool _iceDispatch(IceInternal::Incoming&) final; - /// \endcond + void dispatch(IncomingRequest&, std::function) final; }; /** @@ -176,9 +188,7 @@ namespace Ice std::function error, const Current& current) = 0; - /// \cond INTERNAL - bool _iceDispatch(IceInternal::Incoming&) final; - /// \endcond + void dispatch(IncomingRequest&, std::function) final; }; /** @@ -207,9 +217,8 @@ namespace Ice std::function&)> response, std::function error, const Current& current) = 0; - /// \cond INTERNAL - bool _iceDispatch(IceInternal::Incoming&) final; - /// \endcond + + void dispatch(IncomingRequest&, std::function) final; }; } diff --git a/cpp/include/Ice/ObjectAdapter.h b/cpp/include/Ice/ObjectAdapter.h index 8913f922ff8..4d138c68b86 100644 --- a/cpp/include/Ice/ObjectAdapter.h +++ b/cpp/include/Ice/ObjectAdapter.h @@ -345,6 +345,13 @@ namespace Ice */ virtual ::std::shared_ptr<::Ice::Object> findDefaultServant(const ::std::string& category) const = 0; + /** + * Get the dispatcher associated with this object adapter. This object dispatches incoming requests to the + * servants managed by this object adapter, and takes into account the servant locators. + * @return The dispatcher. This shared_ptr is never null. + */ + virtual ObjectPtr dispatcher() const noexcept = 0; + /** * Create a proxy for the object with the given identity. If this object adapter is configured with an adapter * id, the return value is an indirect proxy that refers to the adapter id. If a replica group id is also diff --git a/cpp/include/Ice/OutgoingResponse.h b/cpp/include/Ice/OutgoingResponse.h new file mode 100644 index 00000000000..0e04c8a5b07 --- /dev/null +++ b/cpp/include/Ice/OutgoingResponse.h @@ -0,0 +1,166 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#ifndef ICE_OUTGOING_RESPONSE_H +#define ICE_OUTGOING_RESPONSE_H + +#include "Current.h" +#include "MarshaledResult.h" +#include "OutputStream.h" + +#include + +namespace Ice +{ + /** + * Represent the status of a response. + * \headerfile Ice/Ice.h + */ + enum class ReplyStatus : std::uint8_t + { + Ok = 0, + UserException = 1, + ObjectNotExist = 2, + FacetNotExist = 3, + OperationNotExist = 4, + UnknownLocalException = 5, + UnknownUserException = 6, + UnknownException = 7 + }; + + /** + * Represent the response to an incoming request. It's the argument to the sendResponse callback accepted by + * Object::dispatch. + * @remarks OutgoingResponse is movable but not copyable. sendResponse wrappers must move the response to the next + * callback. + * @see Object::dispatch + * \headerfile Ice/Ice.h + */ + class ICE_API OutgoingResponse final + { + public: + /** + * Construct an OutgoingResponse object. + * @param replyStatus The status of the response. + * @param exceptionId The ID of the exception, when the response carries an error. + * @param errorMessage The error message, when the response carries an error. + * @param outputStream The output stream that holds the response. Its underlying buffer is adopted by the new + * response. + * @param current A reference to the current object of the request. + */ + OutgoingResponse( + ReplyStatus replyStatus, + std::string exceptionId, + std::string errorMessage, + OutputStream& outputStream, + const Current& current) noexcept; + + /** + * Construct an OutgoingResponse object with the Ok reply status. + * @param outputStream The output stream that holds the response. Its underlying buffer is adopted by the new + * response. + * @param current A reference to the current object of the request. + */ + OutgoingResponse(OutputStream& outputStream, const Current& current) noexcept + : OutgoingResponse(ReplyStatus::Ok, "", "", outputStream, current) + { + } + + /** + * Move constructor. + * @param source The response to move into this new response. + */ + OutgoingResponse(OutgoingResponse&& source) noexcept; + + /** + * Move assignment operator. + * @param source The response to move into this response. + */ + OutgoingResponse& operator=(OutgoingResponse&& source) noexcept; + + OutgoingResponse(const OutgoingResponse&) = delete; + OutgoingResponse& operator=(const OutgoingResponse&) = delete; + + /** + * Return the current object of this response. + * @remarks The response only holds onto a reference for this Current object. The caller keeps the Current + * object alive until the call to sendResponse completes. + */ + const Current& current() const noexcept { return _current; } + + /** + * Return the exception ID of the response. It's empty when replyStatus() is ReplyStatus::Ok. + */ + const std::string& exceptionId() const noexcept { return _exceptionId; } + + /** + * Return the error message of the response. It's empty when replyStatus() is ReplyStatus::Ok. + */ + const std::string& errorMessage() const noexcept { return _errorMessage; } + + /** + * Return the reply status of the response. + */ + ReplyStatus replyStatus() const noexcept { return _replyStatus; } + + /** + * Return the output stream buffer of the response. + */ + OutputStream& outputStream() noexcept { return _outputStream; } + + /** + * Return the number of bytes in the response. + */ + std::int32_t size() const noexcept; + + private: + std::reference_wrapper _current; + std::string _exceptionId; // Empty when _replyStatus == Ok. + std::string _errorMessage; // ditto + OutputStream _outputStream; + ReplyStatus _replyStatus; + }; + + /** + * Create an OutgoingResponse object with the Ok reply status. + * @param marshal A function that writes the payload of the response to the output stream. + * @param current The current object of the incoming request. + * @param format The class format to use when marshaling the response. + * @return The new response. + */ + ICE_API OutgoingResponse makeOutgoingResponse( + std::function marshal, + const Current& current, + FormatType format = FormatType::DefaultFormat) noexcept; + + /** + * Create an OutgoingResponse object with the Ok reply status and an empty payload. + * @param current A reference to the current object of the request. + * @return The new response. + */ + ICE_API OutgoingResponse makeEmptyOutgoingResponse(const Current& current) noexcept; + + /** + * Create an OutgoingResponse object with the Ok or UserException reply status. + * @param ok When true, the reply status is Ok. When false, the reply status is UserException. + * @param encapsulation The payload-encapsulation of the response or the user exception. It should be encoded using + * Current::encoding but this function does not verify it. + * @param current A reference to the current object of the request. + * @return The new response. + */ + ICE_API OutgoingResponse makeOutgoingResponse( + bool ok, + const std::pair& encapsulation, + const Current& current) noexcept; + + /** + * Create an OutgoingResponse object with a reply status other than Ok. + * @param exception The exception to marshal into the response. + * @param current A reference to the current object of the request. + * @return The new response. + */ + ICE_API OutgoingResponse makeOutgoingResponse(std::exception_ptr exception, const Current& current) noexcept; +} + +#endif diff --git a/cpp/include/Ice/ResponseHandlerF.h b/cpp/include/Ice/ResponseHandlerF.h deleted file mode 100644 index 9baa8ec382f..00000000000 --- a/cpp/include/Ice/ResponseHandlerF.h +++ /dev/null @@ -1,17 +0,0 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// - -#ifndef ICE_RESPONSE_HANDLER_F_H -#define ICE_RESPONSE_HANDLER_F_H - -#include "Config.h" -#include - -namespace IceInternal -{ - class ResponseHandler; - using ResponseHandlerPtr = ::std::shared_ptr; -} - -#endif diff --git a/cpp/src/Ice/CollocatedRequestHandler.cpp b/cpp/src/Ice/CollocatedRequestHandler.cpp index fa1121ea590..03202cf77d9 100644 --- a/cpp/src/Ice/CollocatedRequestHandler.cpp +++ b/cpp/src/Ice/CollocatedRequestHandler.cpp @@ -9,7 +9,6 @@ #include #include #include "Ice/OutgoingAsync.h" -#include "Ice/Incoming.h" #include "Endian.h" #include @@ -196,7 +195,7 @@ CollocatedRequestHandler::invokeAsyncRequest(OutgoingAsyncBase* outAsync, int ba } void -CollocatedRequestHandler::sendResponse(int32_t requestId, OutputStream* os, uint8_t) +CollocatedRequestHandler::sendResponse(int32_t requestId, OutputStream* os) { OutgoingAsyncBasePtr outAsync; { @@ -246,7 +245,7 @@ CollocatedRequestHandler::sendNoResponse() } void -CollocatedRequestHandler::invokeException(int32_t requestId, exception_ptr ex, int /*invokeNum*/) +CollocatedRequestHandler::invokeException(int32_t requestId, exception_ptr ex) { handleException(requestId, ex); _adapter->decDirectCount(); @@ -312,7 +311,6 @@ CollocatedRequestHandler::invokeAll(OutputStream* os, int32_t requestId, int32_t } int invokeNum = batchRequestNum > 0 ? batchRequestNum : 1; - ServantManagerPtr servantManager = _adapter->getServantManager(); try { while (invokeNum > 0) @@ -333,21 +331,26 @@ CollocatedRequestHandler::invokeAll(OutputStream* os, int32_t requestId, int32_t break; } - Incoming incoming( - _reference->getInstance().get(), - shared_from_this(), - nullptr, - _adapter, - _response, - 0, - requestId); - incoming.invoke(servantManager, &is); + IncomingRequest request{requestId, nullptr, _adapter, is}; + + try + { + _adapter->dispatcher()->dispatch( + request, + [self = shared_from_this()](OutgoingResponse response) + { self->sendResponse(std::move(response)); }); + } + catch (...) + { + sendResponse(makeOutgoingResponse(current_exception(), request.current())); + } + --invokeNum; } } - catch (const LocalException&) + catch (...) { - invokeException(requestId, current_exception(), invokeNum); // Fatal invocation exception + invokeException(requestId, current_exception()); // Fatal invocation exception } _adapter->decDirectCount(); @@ -384,3 +387,24 @@ CollocatedRequestHandler::handleException(int requestId, std::exception_ptr ex) outAsync->invokeExceptionAsync(); } } + +void +CollocatedRequestHandler::sendResponse(OutgoingResponse response) +{ + try + { + if (_response) + { + sendResponse(response.current().requestId, &response.outputStream()); + } + else + { + sendNoResponse(); + } + } + catch (...) + { + // Fatal invocation exception + invokeException(response.current().requestId, current_exception()); + } +} diff --git a/cpp/src/Ice/CollocatedRequestHandler.h b/cpp/src/Ice/CollocatedRequestHandler.h index 387e2ff68f4..e39524d7b01 100644 --- a/cpp/src/Ice/CollocatedRequestHandler.h +++ b/cpp/src/Ice/CollocatedRequestHandler.h @@ -6,7 +6,6 @@ #define ICE_COLLOCATED_REQUEST_HANDLER_H #include -#include #include #include #include @@ -18,6 +17,7 @@ namespace Ice { class ObjectAdapterI; + class OutgoingResponse; } namespace IceInternal @@ -25,7 +25,8 @@ namespace IceInternal class OutgoingAsyncBase; class OutgoingAsync; - class CollocatedRequestHandler : public RequestHandler, public ResponseHandler + class CollocatedRequestHandler : public RequestHandler, + public std::enable_shared_from_this { public: CollocatedRequestHandler(const ReferencePtr&, const Ice::ObjectAdapterPtr&); @@ -35,10 +36,6 @@ namespace IceInternal virtual void asyncRequestCanceled(const OutgoingAsyncBasePtr&, std::exception_ptr); - virtual void sendResponse(std::int32_t, Ice::OutputStream*, std::uint8_t); - virtual void sendNoResponse(); - virtual void invokeException(std::int32_t, std::exception_ptr, int); - virtual Ice::ConnectionIPtr getConnection(); virtual Ice::ConnectionIPtr waitForConnection(); @@ -48,14 +45,14 @@ namespace IceInternal void invokeAll(Ice::OutputStream*, std::int32_t, std::int32_t); - std::shared_ptr shared_from_this() - { - return std::static_pointer_cast(ResponseHandler::shared_from_this()); - } - private: void handleException(std::int32_t, std::exception_ptr); + void sendResponse(Ice::OutgoingResponse); + void sendResponse(std::int32_t, Ice::OutputStream*); + void sendNoResponse(); + void invokeException(std::int32_t, std::exception_ptr); + const std::shared_ptr _adapter; const bool _hasExecutor; const Ice::LoggerPtr _logger; diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp index 7cd299d10c4..e57cb3a13af 100644 --- a/cpp/src/Ice/ConnectionI.cpp +++ b/cpp/src/Ice/ConnectionI.cpp @@ -13,9 +13,8 @@ #include #include #include -#include // For getThreadPool() and getServantManager(). +#include // For getThreadPool() #include -#include #include #include // For RetryException #include // For createProxy(). @@ -23,6 +22,8 @@ #include #include "CheckIdentity.h" #include "Endian.h" +#include "Ice/IncomingRequest.h" +#include "Ice/OutgoingResponse.h" #ifdef ICE_HAS_BZIP2 # include @@ -56,7 +57,6 @@ namespace uint8_t compress, int32_t requestId, int32_t invokeNum, - const ServantManagerPtr& servantManager, const ObjectAdapterPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, @@ -68,7 +68,6 @@ namespace _compress(compress), _requestId(requestId), _invokeNum(invokeNum), - _servantManager(servantManager), _adapter(adapter), _outAsync(outAsync), _heartbeatCallback(heartbeatCallback), @@ -85,7 +84,6 @@ namespace _compress, _requestId, _invokeNum, - _servantManager, _adapter, _outAsync, _heartbeatCallback, @@ -99,7 +97,6 @@ namespace const uint8_t _compress; const int32_t _requestId; const int32_t _invokeNum; - const ServantManagerPtr _servantManager; const ObjectAdapterPtr _adapter; const OutgoingAsyncBasePtr _outAsync; const HeartbeatCallback _heartbeatCallback; @@ -1223,7 +1220,7 @@ Ice::ConnectionI::setAdapter(const ObjectAdapterPtr& adapter) { if (adapter) { - // Go through the adapter to set the adapter and servant manager on this connection + // Go through the adapter to set the adapter on this connection // to ensure the object adapter is still active. dynamic_cast(adapter.get())->setAdapterOnConnection(shared_from_this()); } @@ -1235,8 +1232,7 @@ Ice::ConnectionI::setAdapter(const ObjectAdapterPtr& adapter) return; } - _adapter = 0; - _servantManager = 0; + _adapter = nullptr; } // @@ -1267,9 +1263,7 @@ Ice::ConnectionI::createProxy(const Identity& ident) const } void -Ice::ConnectionI::setAdapterAndServantManager( - const ObjectAdapterPtr& adapter, - const IceInternal::ServantManagerPtr& servantManager) +Ice::ConnectionI::setAdapterFromAdapter(const ObjectAdapterPtr& adapter) { std::lock_guard lock(_mutex); if (_state <= StateNotValidated || _state >= StateClosing) @@ -1278,7 +1272,6 @@ Ice::ConnectionI::setAdapterAndServantManager( } assert(adapter); // Called by ObjectAdapterI::setAdapterOnConnection _adapter = adapter; - _servantManager = servantManager; } #if defined(ICE_USE_IOCP) @@ -1389,7 +1382,6 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) uint8_t compress = 0; int32_t requestId = 0; int32_t invokeNum = 0; - ServantManagerPtr servantManager; ObjectAdapterPtr adapter; OutgoingAsyncBasePtr outAsync; HeartbeatCallback heartbeatCallback; @@ -1585,7 +1577,6 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) invokeNum, requestId, compress, - servantManager, adapter, outAsync, heartbeatCallback, @@ -1668,7 +1659,6 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) compress, requestId, invokeNum, - servantManager, adapter, outAsync, heartbeatCallback, @@ -1682,7 +1672,6 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) compress, requestId, invokeNum, - servantManager, adapter, outAsync, heartbeatCallback, @@ -1697,7 +1686,6 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) compress, requestId, invokeNum, - servantManager, adapter, outAsync, heartbeatCallback, @@ -1713,7 +1701,6 @@ ConnectionI::dispatch( uint8_t compress, int32_t requestId, int32_t invokeNum, - const ServantManagerPtr& servantManager, const ObjectAdapterPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, @@ -1794,7 +1781,7 @@ ConnectionI::dispatch( // if (invokeNum) { - invokeAll(stream, invokeNum, requestId, compress, servantManager, adapter); + invokeAll(stream, invokeNum, requestId, compress, adapter); // // Don't increase count, the dispatch count is @@ -2166,11 +2153,6 @@ Ice::ConnectionI::ConnectionI( compressionLevel = 9; } - if (adapter) - { - _servantManager = adapter->getServantManager(); - } - if (_monitor && _monitor->getACM().timeout > 0) { _acmLastActivity = chrono::steady_clock::now(); @@ -2571,6 +2553,29 @@ Ice::ConnectionI::sendHeartbeatNow() } } +void +Ice::ConnectionI::sendResponse(OutgoingResponse response, uint8_t compress) +{ + int32_t requestId = response.current().requestId; + bool isTwoWay = !_endpoint->datagram() && requestId != 0; + + try + { + if (isTwoWay) + { + sendResponse(requestId, &response.outputStream(), compress); + } + else + { + sendNoResponse(); + } + } + catch (...) + { + invokeException(requestId, current_exception(), 1); + } +} + bool Ice::ConnectionI::initialize(SocketOperation operation) { @@ -3184,7 +3189,6 @@ Ice::ConnectionI::parseMessage( int32_t& invokeNum, int32_t& requestId, uint8_t& compress, - ServantManagerPtr& servantManager, ObjectAdapterPtr& adapter, OutgoingAsyncBasePtr& outAsync, HeartbeatCallback& heartbeatCallback, @@ -3269,7 +3273,6 @@ Ice::ConnectionI::parseMessage( traceRecv(stream, _logger, _traceLevels); stream.read(requestId); invokeNum = 1; - servantManager = _servantManager; adapter = _adapter; ++dispatchCount; } @@ -3295,7 +3298,6 @@ Ice::ConnectionI::parseMessage( invokeNum = 0; throw UnmarshalOutOfBoundsException(__FILE__, __LINE__); } - servantManager = _servantManager; adapter = _adapter; dispatchCount += invokeNum; } @@ -3418,7 +3420,6 @@ Ice::ConnectionI::invokeAll( int32_t invokeNum, int32_t requestId, uint8_t compress, - const ServantManagerPtr& servantManager, const ObjectAdapterPtr& adapter) { // @@ -3431,31 +3432,41 @@ Ice::ConnectionI::invokeAll( while (invokeNum > 0) { // - // Prepare the dispatch. + // Start of the dispatch pipeline. // - bool response = !_endpoint->datagram() && requestId != 0; - assert(!response || invokeNum == 1); - - Incoming incoming( - _instance.get(), - shared_from_this(), - shared_from_this(), - adapter, - response, - compress, - requestId); - // - // Dispatch the incoming request. - // - incoming.invoke(servantManager, &stream); + IncomingRequest request{requestId, shared_from_this(), adapter, stream}; + + if (adapter) + { + try + { + adapter->dispatcher()->dispatch( + request, + [self = shared_from_this(), compress](OutgoingResponse response) + { self->sendResponse(std::move(response), compress); }); + } + catch (...) + { + sendResponse(makeOutgoingResponse(current_exception(), request.current()), 0); + } + } + else + { + // Received request on a connection without an object adapter. + sendResponse( + makeOutgoingResponse( + make_exception_ptr(ObjectNotExistException{__FILE__, __LINE__}), + request.current()), + 0); + } --invokeNum; } stream.clear(); } - catch (const LocalException&) + catch (...) { invokeException(requestId, current_exception(), invokeNum); // Fatal invocation exception } diff --git a/cpp/src/Ice/ConnectionI.h b/cpp/src/Ice/ConnectionI.h index df11bfe7b5b..99b521ed01a 100644 --- a/cpp/src/Ice/ConnectionI.h +++ b/cpp/src/Ice/ConnectionI.h @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -23,7 +22,6 @@ #include "Ice/OutgoingAsync.h" #include #include -#include #include #include #include @@ -49,10 +47,7 @@ namespace Ice class LocalException; class ObjectAdapterI; - class ConnectionI : public Connection, - public IceInternal::EventHandler, - public IceInternal::ResponseHandler, - public IceInternal::CancellationHandler + class ConnectionI : public Connection, public IceInternal::EventHandler, public IceInternal::CancellationHandler { class Observer : public IceInternal::ObserverHelperT { @@ -174,10 +169,6 @@ namespace Ice virtual void asyncRequestCanceled(const IceInternal::OutgoingAsyncBasePtr&, std::exception_ptr); - virtual void sendResponse(std::int32_t, Ice::OutputStream*, std::uint8_t); - virtual void sendNoResponse(); - virtual void invokeException(std::int32_t, std::exception_ptr, int); - IceInternal::EndpointIPtr endpoint() const; IceInternal::ConnectorPtr connector() const; @@ -186,7 +177,7 @@ namespace Ice virtual EndpointPtr getEndpoint() const noexcept; // From Connection. virtual ObjectPrx createProxy(const Identity& ident) const; // From Connection. - void setAdapterAndServantManager(const ObjectAdapterPtr&, const IceInternal::ServantManagerPtr&); + void setAdapterFromAdapter(const ObjectAdapterPtr&); // From ObjectAdapterI. // // Operations from EventHandler @@ -217,7 +208,6 @@ namespace Ice std::uint8_t, std::int32_t, std::int32_t, - const IceInternal::ServantManagerPtr&, const ObjectAdapterPtr&, const IceInternal::OutgoingAsyncBasePtr&, const HeartbeatCallback&, @@ -268,6 +258,11 @@ namespace Ice void initiateShutdown(); void sendHeartbeatNow(); + void sendResponse(OutgoingResponse, std::uint8_t compress); + void sendResponse(std::int32_t, Ice::OutputStream*, std::uint8_t); + void sendNoResponse(); + void invokeException(std::int32_t, std::exception_ptr, int); + bool initialize(IceInternal::SocketOperation = IceInternal::SocketOperationNone); bool validate(IceInternal::SocketOperation = IceInternal::SocketOperationNone); IceInternal::SocketOperation sendNextMessage(std::vector&); @@ -283,19 +278,12 @@ namespace Ice std::int32_t&, std::int32_t&, std::uint8_t&, - IceInternal::ServantManagerPtr&, ObjectAdapterPtr&, IceInternal::OutgoingAsyncBasePtr&, HeartbeatCallback&, int&); - void invokeAll( - Ice::InputStream&, - std::int32_t, - std::int32_t, - std::uint8_t, - const IceInternal::ServantManagerPtr&, - const ObjectAdapterPtr&); + void invokeAll(Ice::InputStream&, std::int32_t, std::int32_t, std::uint8_t, const ObjectAdapterPtr&); void scheduleTimeout(IceInternal::SocketOperation status); void unscheduleTimeout(IceInternal::SocketOperation status); @@ -320,7 +308,6 @@ namespace Ice mutable Ice::ConnectionInfoPtr _info; ObjectAdapterPtr _adapter; - IceInternal::ServantManagerPtr _servantManager; const bool _hasExecutor; const LoggerPtr _logger; diff --git a/cpp/src/Ice/Exception.cpp b/cpp/src/Ice/Exception.cpp index 900adcf279c..b7ea59725ca 100644 --- a/cpp/src/Ice/Exception.cpp +++ b/cpp/src/Ice/Exception.cpp @@ -747,13 +747,6 @@ Ice::FixedProxyException::ice_print(ostream& out) const out << ":\nfixed proxy exception"; } -void -Ice::ResponseSentException::ice_print(ostream& out) const -{ - Exception::ice_print(out); - out << ":\nresponse sent exception"; -} - void Ice::CFNetworkException::ice_print(ostream& out) const { diff --git a/cpp/src/Ice/Incoming.cpp b/cpp/src/Ice/Incoming.cpp deleted file mode 100644 index 44a15cfc52f..00000000000 --- a/cpp/src/Ice/Incoming.cpp +++ /dev/null @@ -1,672 +0,0 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -using namespace std; -using namespace Ice; -using namespace Ice::Instrumentation; -using namespace IceInternal; - -namespace IceUtilInternal -{ - extern bool printStackTraces; -} - -Incoming::Incoming( - Instance* instance, - ResponseHandlerPtr responseHandler, - Ice::ConnectionPtr connection, - ObjectAdapterPtr adapter, - bool twoWay, - uint8_t compress, - int32_t requestId) - : _isTwoWay(twoWay), - _compress(compress), - _format(Ice::FormatType::DefaultFormat), - _os(instance, Ice::currentProtocolEncoding), - _responseHandler(std::move(responseHandler)), - _is(nullptr) -{ - _current.adapter = std::move(adapter); - _current.con = std::move(connection); - _current.requestId = requestId; - _current.encoding.major = 0; - _current.encoding.minor = 0; -} - -Incoming::Incoming(Incoming&& other) - : _current(std::move(other._current)), - _servant(std::move(other._servant)), - _locator(std::move(other._locator)), - _cookie(std::move(other._cookie)), - _isTwoWay(std::move(other._isTwoWay)), - _compress(std::move(other._compress)), - _format(std::move(other._format)), - _os(other._os.instance(), Ice::currentProtocolEncoding), // TODO: make movable - _responseHandler(std::move(other._responseHandler)), - _is(std::move(other._is)) // just copies the pointer -{ - _observer.adopt(other._observer); // TODO: make movable -} - -OutputStream* -Incoming::startWriteParams() -{ - if (!_isTwoWay) - { - throw MarshalException(__FILE__, __LINE__, "can't marshal out parameters for oneway dispatch"); - } - - assert(_current.encoding >= Ice::Encoding_1_0); // Encoding for reply is known. - - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - _os.write(replyOK); - _os.startEncapsulation(_current.encoding, _format); - return &_os; -} - -void -Incoming::endWriteParams() -{ - if (_isTwoWay) - { - _os.endEncapsulation(); - } -} - -void -Incoming::writeEmptyParams() -{ - if (_isTwoWay) - { - assert(_current.encoding >= Ice::Encoding_1_0); // Encoding for reply is known. - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - _os.write(replyOK); - _os.writeEmptyEncapsulation(_current.encoding); - } -} - -void -Incoming::writeParamEncaps(const uint8_t* v, int32_t sz, bool ok) -{ - if (!ok) - { - _observer.userException(); - } - - if (_isTwoWay) - { - assert(_current.encoding >= Ice::Encoding_1_0); // Encoding for reply is known. - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - _os.write(ok ? replyOK : replyUserException); - if (sz == 0) - { - _os.writeEmptyEncapsulation(_current.encoding); - } - else - { - _os.writeEncapsulation(v, sz); - } - } -} - -void -Incoming::setMarshaledResult(Ice::MarshaledResult&& result) -{ - result.swap(_os); -} - -void -Incoming::sendResponse() -{ - try - { - if (_locator && !servantLocatorFinished()) - { - return; - } - - assert(_responseHandler); - if (_isTwoWay) - { - _observer.reply(static_cast(_os.b.size() - headerSize - 4)); - _responseHandler->sendResponse(_current.requestId, &_os, _compress); - } - else - { - _responseHandler->sendNoResponse(); - } - } - catch (const LocalException&) - { - _responseHandler->invokeException(_current.requestId, current_exception(), 1); // Fatal invocation exception - } - - _observer.detach(); - _responseHandler = nullptr; -} - -void -Incoming::sendException(std::exception_ptr exc) -{ - try - { - if (_locator && !servantLocatorFinished()) - { - return; - } - handleException(exc); - } - catch (const LocalException&) - { - _responseHandler->invokeException(_current.requestId, current_exception(), 1); // Fatal invocation exception - } -} - -void -Incoming::response() -{ - writeEmptyParams(); - completed(); -} - -void -Incoming::response(MarshaledResult&& result) -{ - setMarshaledResult(std::move(result)); - completed(); -} - -void -Incoming::warning(const Exception& ex) const -{ - Warning out(_os.instance()->initializationData().logger); - - ToStringMode toStringMode = _os.instance()->toStringMode(); - - out << "dispatch exception: " << ex; - out << "\nidentity: " << identityToString(_current.id, toStringMode); - out << "\nfacet: "; - out << escapeString(_current.facet, "", toStringMode); - out << "\noperation: " << _current.operation; - - if (_current.con) - { - try - { - for (Ice::ConnectionInfoPtr connInfo = _current.con->getInfo(); connInfo; connInfo = connInfo->underlying) - { - Ice::IPConnectionInfoPtr ipConnInfo = dynamic_pointer_cast(connInfo); - if (ipConnInfo) - { - out << "\nremote host: " << ipConnInfo->remoteAddress << " remote port: " << ipConnInfo->remotePort; - break; - } - } - } - catch (const Ice::LocalException&) - { - // Ignore. - } - } -} - -void -Incoming::warning(std::exception_ptr ex) const -{ - Warning out(_os.instance()->initializationData().logger); - ToStringMode toStringMode = _os.instance()->toStringMode(); - - out << "dispatch exception: "; - - try - { - rethrow_exception(ex); - } - catch (const std::exception& e) - { - out << "std::exception: " << e.what(); - } - catch (...) - { - out << "unknown c++ exception"; - } - out << "\nidentity: " << identityToString(_current.id, toStringMode); - out << "\nfacet: " << escapeString(_current.facet, "", toStringMode); - out << "\noperation: " << _current.operation; - - if (_current.con) - { - for (Ice::ConnectionInfoPtr connInfo = _current.con->getInfo(); connInfo; connInfo = connInfo->underlying) - { - Ice::IPConnectionInfoPtr ipConnInfo = dynamic_pointer_cast(connInfo); - if (ipConnInfo) - { - out << "\nremote host: " << ipConnInfo->remoteAddress << " remote port: " << ipConnInfo->remotePort; - break; - } - } - } -} - -bool -Incoming::servantLocatorFinished() -{ - assert(_locator && _servant); - try - { - _locator->finished(_current, _servant, _cookie); - return true; - } - catch (...) - { - handleException(current_exception()); - } - return false; -} - -void -Incoming::handleException(std::exception_ptr exc) -{ - assert(_responseHandler); - - // Reset the stream, it's possible the marshalling of the response failed and left - // the stream in an unknown state. - _os.clear(); - _os.b.clear(); - - try - { - rethrow_exception(exc); - } - catch (RequestFailedException& rfe) - { - if (rfe.id.name.empty()) - { - rfe.id = _current.id; - } - - if (rfe.facet.empty() && !_current.facet.empty()) - { - rfe.facet = _current.facet; - } - - if (rfe.operation.empty() && !_current.operation.empty()) - { - rfe.operation = _current.operation; - } - - if (_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 1) - { - warning(rfe); - } - - if (_observer) - { - _observer.failed(rfe.ice_id()); - } - - if (_isTwoWay) - { - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - if (dynamic_cast(&rfe)) - { - _os.write(replyObjectNotExist); - } - else if (dynamic_cast(&rfe)) - { - _os.write(replyFacetNotExist); - } - else if (dynamic_cast(&rfe)) - { - _os.write(replyOperationNotExist); - } - else - { - assert(false); - } - - _os.write(rfe.id); - - // - // For compatibility with the old FacetPath. - // - if (rfe.facet.empty()) - { - _os.write(static_cast(0), static_cast(0)); - } - else - { - _os.write(&rfe.facet, &rfe.facet + 1); - } - - _os.write(rfe.operation, false); - - _observer.reply(static_cast(_os.b.size() - headerSize - 4)); - _responseHandler->sendResponse(_current.requestId, &_os, _compress); - } - else - { - _responseHandler->sendNoResponse(); - } - } - catch (const UserException& uex) - { - _observer.userException(); - - // - // The operation may have already marshaled a reply; we must overwrite that reply. - // - if (_isTwoWay) - { - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - _os.write(replyUserException); - _os.startEncapsulation(_current.encoding, _format); - _os.write(uex); - _os.endEncapsulation(); - _observer.reply(static_cast(_os.b.size() - headerSize - 4)); - _responseHandler->sendResponse(_current.requestId, &_os, _compress); - } - else - { - _responseHandler->sendNoResponse(); - } - } - catch (const Exception& ex) - { - if (_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0) - { - warning(ex); - } - - if (_observer) - { - _observer.failed(ex.ice_id()); - } - - if (_isTwoWay) - { - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - if (const UnknownLocalException* ule = dynamic_cast(&ex)) - { - _os.write(replyUnknownLocalException); - _os.write(ule->unknown, false); - } - else if (const UnknownUserException* uue = dynamic_cast(&ex)) - { - _os.write(replyUnknownUserException); - _os.write(uue->unknown, false); - } - else if (const UnknownException* ue = dynamic_cast(&ex)) - { - _os.write(replyUnknownException); - _os.write(ue->unknown, false); - } - else if (const LocalException* le = dynamic_cast(&ex)) - { - _os.write(replyUnknownLocalException); - ostringstream str; - str << *le; - if (IceUtilInternal::printStackTraces) - { - str << '\n' << ex.ice_stackTrace(); - } - _os.write(str.str(), false); - } - else if (const UserException* use = dynamic_cast(&ex)) - { - _os.write(replyUnknownUserException); - ostringstream str; - str << *use; - if (IceUtilInternal::printStackTraces) - { - str << '\n' << ex.ice_stackTrace(); - } - _os.write(str.str(), false); - } - else - { - _os.write(replyUnknownException); - ostringstream str; - str << ex; - if (IceUtilInternal::printStackTraces) - { - str << '\n' << ex.ice_stackTrace(); - } - _os.write(str.str(), false); - } - - _observer.reply(static_cast(_os.b.size() - headerSize - 4)); - _responseHandler->sendResponse(_current.requestId, &_os, _compress); - } - else - { - _responseHandler->sendNoResponse(); - } - } - catch (...) - { - string exceptionId = getExceptionId(exc); - - if (_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0) - { - warning(exc); - } - - if (_observer) - { - _observer.failed(exceptionId); - } - - if (_isTwoWay) - { - _os.writeBlob(replyHdr, sizeof(replyHdr)); - _os.write(_current.requestId); - _os.write(replyUnknownException); - ostringstream str; - str << "c++ exception: " << exceptionId; - _os.write(str.str(), false); - - _observer.reply(static_cast(_os.b.size() - headerSize - 4)); - _responseHandler->sendResponse(_current.requestId, &_os, _compress); - } - else - { - _responseHandler->sendNoResponse(); - } - } - - _observer.detach(); - _responseHandler = nullptr; -} - -void -Incoming::invoke(const ServantManagerPtr& servantManager, InputStream* stream) -{ - _is = stream; - - InputStream::Container::iterator start = _is->i; - - // - // Read the current. - // - _is->read(_current.id); - - // - // For compatibility with the old FacetPath. - // - string facet; - { - vector facetPath; - _is->read(facetPath); - if (!facetPath.empty()) - { - if (facetPath.size() > 1) - { - throw MarshalException(__FILE__, __LINE__); - } - facet.swap(facetPath[0]); - } - } - _current.facet.swap(facet); - - _is->read(_current.operation, false); - - uint8_t b; - _is->read(b); - _current.mode = static_cast(b); - - int32_t sz = _is->readSize(); - while (sz--) - { - pair pr; - _is->read(const_cast(pr.first)); - _is->read(pr.second); - _current.ctx.insert(_current.ctx.end(), pr); - } - - const CommunicatorObserverPtr& obsv = _is->instance()->initializationData().observer; - if (obsv) - { - // Read the parameter encapsulation size. - int32_t encapsSize; - _is->read(encapsSize); - _is->i -= 4; - - _observer.attach(obsv->getDispatchObserver(_current, static_cast(_is->i - start + encapsSize))); - } - - // - // Don't put the code above into the try block below. Exceptions - // in the code above are considered fatal, and must propagate to - // the caller of this operation. - // - - if (servantManager) - { - _servant = servantManager->findServant(_current.id, _current.facet); - if (!_servant) - { - _locator = servantManager->findServantLocator(_current.id.category); - if (!_locator && !_current.id.category.empty()) - { - _locator = servantManager->findServantLocator(""); - } - - if (_locator) - { - try - { - _servant = _locator->locate(_current, _cookie); - } - catch (...) - { - skipReadParams(); // Required for batch requests. - handleException(current_exception()); - return; - } - } - } - } - - if (!_servant) - { - try - { - if (servantManager && servantManager->hasServant(_current.id)) - { - throw FacetNotExistException(__FILE__, __LINE__, _current.id, _current.facet, _current.operation); - } - else - { - throw ObjectNotExistException(__FILE__, __LINE__, _current.id, _current.facet, _current.operation); - } - } - catch (...) - { - skipReadParams(); // Required for batch requests - handleException(current_exception()); - return; - } - } - - try - { - // Dispatch the request to the servant. - if (_servant->_iceDispatch(*this)) - { - // If the request was dispatched synchronously, send the response. - sendResponse(); - } - } - catch (...) - { - // An async dispatch is not allowed to throw any exception because it moves "this" memory into a new Incoming - // object. - sendException(current_exception()); - } -} - -void -Incoming::completed() -{ - setResponseSent(); - sendResponse(); -} - -void -Incoming::completed(exception_ptr ex) -{ - setResponseSent(); - sendException(ex); -} - -void -Incoming::failed(exception_ptr ex) noexcept -{ - try - { - completed(ex); - } - catch (...) - { - // Ignore all exceptions. The caller in the dispatch thread can't handle any exception because its memory - // was moved to a new "async" incoming object. - } -} - -void -Incoming::setResponseSent() -{ - if (_responseSent) - { - // The application must not call the callbacks twice, or call a callback after throwing an exception from the - // dispatch thread. - throw ResponseSentException(__FILE__, __LINE__); - } - else - { - _responseSent = true; - } -} diff --git a/cpp/src/Ice/IncomingRequest.cpp b/cpp/src/Ice/IncomingRequest.cpp new file mode 100644 index 00000000000..b43662377bd --- /dev/null +++ b/cpp/src/Ice/IncomingRequest.cpp @@ -0,0 +1,66 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#include "Ice/IncomingRequest.h" +#include "Ice/InputStream.h" +#include "Ice/LocalException.h" + +using namespace std; +using namespace Ice; +using namespace IceInternal; + +IncomingRequest::IncomingRequest( + int32_t requestId, + ConnectionPtr connection, + ObjectAdapterPtr adapter, + InputStream& inputStream) + : _inputStream(inputStream) +{ + _current.adapter = std::move(adapter); + _current.con = std::move(connection); + _current.requestId = requestId; + + // Read everything else from the input stream. + auto start = inputStream.i; + inputStream.read(_current.id); + + vector facetPath; + inputStream.read(facetPath); + if (!facetPath.empty()) + { + if (facetPath.size() > 1) + { + throw MarshalException{__FILE__, __LINE__}; + } + _current.facet = facetPath[0]; + } + + inputStream.read(_current.operation, false); + + uint8_t mode; + inputStream.read(mode); + _current.mode = static_cast(mode); + + int32_t sz = inputStream.readSize(); + while (sz--) + { + string key; + string value; + inputStream.read(key); + inputStream.read(value); + _current.ctx.emplace(std::move(key), std::move(value)); + } + + int32_t encapsulationSize; + inputStream.read(encapsulationSize); + EncodingVersion encoding; + inputStream.read(encoding.major); + inputStream.read(encoding.minor); + _current.encoding = encoding; + + // Rewind to the start of the encapsulation + inputStream.i -= 6; + + _requestSize = static_cast(inputStream.i - start) + encapsulationSize; +} diff --git a/cpp/src/Ice/LocalException.cpp b/cpp/src/Ice/LocalException.cpp index b9df06f1332..c9c1fe630c0 100644 --- a/cpp/src/Ice/LocalException.cpp +++ b/cpp/src/Ice/LocalException.cpp @@ -603,12 +603,3 @@ Ice::FixedProxyException::ice_staticId() static constexpr std::string_view typeId = "::Ice::FixedProxyException"; return typeId; } - -Ice::ResponseSentException::~ResponseSentException() {} - -std::string_view -Ice::ResponseSentException::ice_staticId() -{ - static constexpr std::string_view typeId = "::Ice::ResponseSentException"; - return typeId; -} diff --git a/cpp/src/Ice/LoggerMiddleware.cpp b/cpp/src/Ice/LoggerMiddleware.cpp new file mode 100644 index 00000000000..81cb8eb4e76 --- /dev/null +++ b/cpp/src/Ice/LoggerMiddleware.cpp @@ -0,0 +1,119 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#include "LoggerMiddleware.h" +#include "Ice/StringUtil.h" + +using namespace std; +using namespace Ice; +using namespace IceInternal; + +LoggerMiddleware::LoggerMiddleware(Ice::ObjectPtr next, LoggerPtr logger, int warningLevel, ToStringMode toStringMode) + : _next(std::move(next)), + _logger(std::move(logger)), + _warningLevel(warningLevel), + _toStringMode(toStringMode) +{ + assert(_next); + assert(_logger); + assert(_warningLevel > 0); +} + +void +LoggerMiddleware::dispatch(Ice::IncomingRequest& request, function sendResponse) +{ + try + { + _next->dispatch( + request, + [sendResponse = std::move(sendResponse), self = shared_from_this()](OutgoingResponse response) + { + switch (response.replyStatus()) + { + case ReplyStatus::Ok: + case ReplyStatus::UserException: + // no warning + break; + case ReplyStatus::ObjectNotExist: + case ReplyStatus::FacetNotExist: + case ReplyStatus::OperationNotExist: + if (self->_warningLevel > 1) + { + self->warning(response.errorMessage(), response.current()); + } + break; + + default: + self->warning(response.errorMessage(), response.current()); + break; + } + sendResponse(std::move(response)); + }); + } + catch (const UserException&) + { + // No warning. + throw; + } + catch (const RequestFailedException& ex) + { + if (_warningLevel > 1) + { + warning(ex, request.current()); + } + throw; + } + catch (const Ice::Exception& ex) + { + warning(ex, request.current()); + throw; + } + catch (const std::exception& ex) + { + warning(ex.what(), request.current()); + throw; + } + catch (...) + { + warning("c++ exception", request.current()); + throw; + } +} + +void +LoggerMiddleware::warning(const Exception& ex, const Current& current) const noexcept +{ + Warning out(_logger); + out << "dispatch exception: " << ex; + warning(out, current); +} + +void +LoggerMiddleware::warning(const string& errorMessage, const Current& current) const noexcept +{ + Warning out(_logger); + out << "dispatch exception: " << errorMessage; + warning(out, current); +} + +void +LoggerMiddleware::warning(Warning& out, const Current& current) const noexcept +{ + out << "\nidentity: " << identityToString(current.id, _toStringMode); + out << "\nfacet: " << escapeString(current.facet, "", _toStringMode); + out << "\noperation: " << current.operation; + + if (current.con) + { + for (Ice::ConnectionInfoPtr connInfo = current.con->getInfo(); connInfo; connInfo = connInfo->underlying) + { + Ice::IPConnectionInfoPtr ipConnInfo = dynamic_pointer_cast(connInfo); + if (ipConnInfo) + { + out << "\nremote host: " << ipConnInfo->remoteAddress << " remote port: " << ipConnInfo->remotePort; + break; + } + } + } +} diff --git a/cpp/src/Ice/LoggerMiddleware.h b/cpp/src/Ice/LoggerMiddleware.h new file mode 100644 index 00000000000..427549e3807 --- /dev/null +++ b/cpp/src/Ice/LoggerMiddleware.h @@ -0,0 +1,35 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#ifndef ICE_LOGGER_MIDDLEWARE_H +#define ICE_LOGGER_MIDDLEWARE_H + +#include "Ice/Initialize.h" // for ToStringMode +#include "Ice/Object.h" +#include "Ice/Logger.h" +#include "Ice/LoggerUtil.h" + +namespace IceInternal +{ + /// A middleware that logs warnings for failed dispatches. + class LoggerMiddleware final : public Ice::Object, public std::enable_shared_from_this + { + public: + LoggerMiddleware(Ice::ObjectPtr next, Ice::LoggerPtr logger, int warningLevel, Ice::ToStringMode toStringMode); + + void dispatch(Ice::IncomingRequest&, std::function) final; + + private: + void warning(const Ice::Exception&, const Ice::Current&) const noexcept; + void warning(const std::string&, const Ice::Current&) const noexcept; + void warning(Ice::Warning&, const Ice::Current&) const noexcept; + + Ice::ObjectPtr _next; + Ice::LoggerPtr _logger; + int _warningLevel; + Ice::ToStringMode _toStringMode; + }; +} + +#endif diff --git a/cpp/src/Ice/MarshaledResult.cpp b/cpp/src/Ice/MarshaledResult.cpp index d604a232250..fbccf2f7a84 100644 --- a/cpp/src/Ice/MarshaledResult.cpp +++ b/cpp/src/Ice/MarshaledResult.cpp @@ -10,7 +10,7 @@ using namespace std; using namespace Ice; using namespace IceInternal; -MarshaledResult::MarshaledResult(MarshaledResult&& rhs) { swap(rhs._ostr); } +MarshaledResult::MarshaledResult(MarshaledResult&& rhs) { _ostr.swap(rhs._ostr); } MarshaledResult::MarshaledResult(const Current& current) : // currentProtocolEncoding because we're writing the protocol header. @@ -24,12 +24,6 @@ MarshaledResult::MarshaledResult(const Current& current) MarshaledResult& MarshaledResult::operator=(MarshaledResult&& rhs) { - swap(rhs._ostr); + _ostr.swap(rhs._ostr); return *this; } - -void -MarshaledResult::swap(OutputStream& other) -{ - _ostr.swap(other); -} diff --git a/cpp/src/Ice/Object.cpp b/cpp/src/Ice/Object.cpp index 234743a600a..58be2faf8c2 100644 --- a/cpp/src/Ice/Object.cpp +++ b/cpp/src/Ice/Object.cpp @@ -3,7 +3,7 @@ // #include "Ice/Object.h" -#include "Ice/Incoming.h" +#include "Ice/AsyncResponseHandler.h" #include "Ice/LocalException.h" #include "Ice/SlicedData.h" @@ -49,94 +49,99 @@ Ice::Object::ice_staticId() return typeId; } -bool -Ice::Object::_iceD_ice_isA(Incoming& incoming) +void +Ice::Object::_iceD_ice_isA(IncomingRequest& request, function sendResponse) { - InputStream* istr = incoming.startReadParams(); + InputStream* istr = &request.inputStream(); + istr->startEncapsulation(); string iceP_id; istr->read(iceP_id, false); - incoming.endReadParams(); - bool ret = ice_isA(std::move(iceP_id), incoming.current()); - OutputStream* ostr = incoming.startWriteParams(); - ostr->write(ret); - incoming.endWriteParams(); - return true; + istr->endEncapsulation(); + + bool ret = ice_isA(std::move(iceP_id), request.current()); + sendResponse(makeOutgoingResponse([&](OutputStream* ostr) { ostr->write(ret); }, request.current())); } -bool -Ice::Object::_iceD_ice_ping(Incoming& incoming) +void +Ice::Object::_iceD_ice_ping(IncomingRequest& request, function sendResponse) { - incoming.readEmptyParams(); - ice_ping(incoming.current()); - incoming.writeEmptyParams(); - return true; + request.inputStream().skipEmptyEncapsulation(); + ice_ping(request.current()); + sendResponse(makeEmptyOutgoingResponse(request.current())); } -bool -Ice::Object::_iceD_ice_ids(Incoming& incoming) +void +Ice::Object::_iceD_ice_ids(IncomingRequest& request, function sendResponse) { - incoming.readEmptyParams(); - vector ret = ice_ids(incoming.current()); - OutputStream* ostr = incoming.startWriteParams(); - if (ret.empty()) - { - ostr->write(ret); - } - else - { - ostr->write(&ret[0], &ret[0] + ret.size(), false); - } - incoming.endWriteParams(); - return true; + request.inputStream().skipEmptyEncapsulation(); + vector ret = ice_ids(request.current()); + + sendResponse(makeOutgoingResponse( + [&](OutputStream* ostr) + { + if (ret.empty()) + { + ostr->write(ret); + } + else + { + ostr->write(&ret[0], &ret[0] + ret.size(), false); + } + }, + request.current())); } -bool -Ice::Object::_iceD_ice_id(Incoming& incoming) +void +Ice::Object::_iceD_ice_id(IncomingRequest& request, function sendResponse) { - incoming.readEmptyParams(); - string ret = ice_id(incoming.current()); - OutputStream* ostr = incoming.startWriteParams(); - ostr->write(ret, false); - incoming.endWriteParams(); - return true; + request.inputStream().skipEmptyEncapsulation(); + string ret = ice_id(request.current()); + + sendResponse(makeOutgoingResponse([&](OutputStream* ostr) { ostr->write(ret, false); }, request.current())); } -bool -Ice::Object::_iceDispatch(Incoming& incoming) +void +Ice::Object::dispatch(IncomingRequest& request, std::function sendResponse) { static constexpr string_view allOperations[] = {"ice_id", "ice_ids", "ice_isA", "ice_ping"}; - const Current& current = incoming.current(); + const Current& current = request.current(); pair r = equal_range(allOperations, allOperations + 4, current.operation); if (r.first == r.second) { - throw OperationNotExistException(__FILE__, __LINE__, current.id, current.facet, current.operation); + sendResponse(makeOutgoingResponse(make_exception_ptr(OperationNotExistException(__FILE__, __LINE__)), current)); + return; } switch (r.first - allOperations) { case 0: { - return _iceD_ice_id(incoming); + _iceD_ice_id(request, std::move(sendResponse)); + break; } case 1: { - return _iceD_ice_ids(incoming); + _iceD_ice_ids(request, std::move(sendResponse)); + break; } case 2: { - return _iceD_ice_isA(incoming); + _iceD_ice_isA(request, std::move(sendResponse)); + break; } case 3: { - return _iceD_ice_ping(incoming); + _iceD_ice_ping(request, std::move(sendResponse)); + break; } default: { assert(false); - throw OperationNotExistException(__FILE__, __LINE__, current.id, current.facet, current.operation); + sendResponse( + makeOutgoingResponse(make_exception_ptr(OperationNotExistException(__FILE__, __LINE__)), current)); } } } @@ -186,103 +191,97 @@ Ice::Object::_iceCheckMode(OperationMode expected, OperationMode received) } } -bool -Ice::Blobject::_iceDispatch(Incoming& incoming) +void +Ice::Blobject::dispatch(IncomingRequest& request, std::function sendResponse) { - const Current& current = incoming.current(); + const Current& current = request.current(); const uint8_t* inEncaps; int32_t sz; - incoming.readParamEncaps(inEncaps, sz); + request.inputStream().readEncapsulation(inEncaps, sz); vector outEncaps; bool ok = ice_invoke(vector(inEncaps, inEncaps + sz), outEncaps, current); + if (outEncaps.empty()) { - incoming.writeParamEncaps(0, 0, ok); + sendResponse(makeOutgoingResponse(ok, {nullptr, nullptr}, current)); } else { - incoming.writeParamEncaps(&outEncaps[0], static_cast(outEncaps.size()), ok); + sendResponse(makeOutgoingResponse(ok, {outEncaps.data(), outEncaps.data() + outEncaps.size()}, current)); } - return true; } -bool -Ice::BlobjectArray::_iceDispatch(Incoming& incoming) +void +Ice::BlobjectArray::dispatch(IncomingRequest& request, std::function sendResponse) { - const Current& current = incoming.current(); + const Current& current = request.current(); pair inEncaps; int32_t sz; - incoming.readParamEncaps(inEncaps.first, sz); + request.inputStream().readEncapsulation(inEncaps.first, sz); inEncaps.second = inEncaps.first + sz; vector outEncaps; bool ok = ice_invoke(inEncaps, outEncaps, current); + if (outEncaps.empty()) { - incoming.writeParamEncaps(0, 0, ok); + sendResponse(makeOutgoingResponse(ok, {nullptr, nullptr}, current)); } else { - incoming.writeParamEncaps(&outEncaps[0], static_cast(outEncaps.size()), ok); + sendResponse(makeOutgoingResponse(ok, {outEncaps.data(), outEncaps.data() + outEncaps.size()}, current)); } - return true; } -bool -Ice::BlobjectAsync::_iceDispatch(Incoming& incoming) +void +Ice::BlobjectAsync::dispatch(IncomingRequest& request, std::function sendResponse) { const uint8_t* inEncaps; int32_t sz; - incoming.readParamEncaps(inEncaps, sz); - auto incomingPtr = make_shared(std::move(incoming)); + request.inputStream().readEncapsulation(inEncaps, sz); + auto responseHandler = make_shared(std::move(sendResponse), request.current()); try { ice_invokeAsync( - vector(inEncaps, inEncaps + sz), - [incomingPtr](bool ok, const vector& outEncaps) + vector{inEncaps, inEncaps + sz}, + [responseHandler](bool ok, const vector& outEncaps) { if (outEncaps.empty()) { - incomingPtr->writeParamEncaps(0, 0, ok); + responseHandler->sendResponse(ok, {nullptr, nullptr}); } else { - incomingPtr->writeParamEncaps(&outEncaps[0], static_cast(outEncaps.size()), ok); + responseHandler->sendResponse(ok, {outEncaps.data(), outEncaps.data() + outEncaps.size()}); } - incomingPtr->completed(); }, - [incomingPtr](std::exception_ptr ex) { incomingPtr->completed(ex); }, - incomingPtr->current()); + [responseHandler](std::exception_ptr ex) { responseHandler->sendException(ex); }, + responseHandler->current()); } catch (...) { - incomingPtr->failed(std::current_exception()); + responseHandler->sendException(std::current_exception()); } - return false; } -bool -Ice::BlobjectArrayAsync::_iceDispatch(Incoming& incoming) +void +Ice::BlobjectArrayAsync::dispatch(IncomingRequest& request, std::function sendResponse) { pair inEncaps; int32_t sz; - incoming.readParamEncaps(inEncaps.first, sz); + request.inputStream().readEncapsulation(inEncaps.first, sz); inEncaps.second = inEncaps.first + sz; - auto incomingPtr = make_shared(std::move(incoming)); + auto responseHandler = make_shared(std::move(sendResponse), request.current()); try { ice_invokeAsync( inEncaps, - [incomingPtr](bool ok, const pair& outE) - { - incomingPtr->writeParamEncaps(outE.first, static_cast(outE.second - outE.first), ok); - incomingPtr->completed(); - }, - [incomingPtr](std::exception_ptr ex) { incomingPtr->completed(ex); }, - incomingPtr->current()); + [responseHandler](bool ok, const pair& outE) + { responseHandler->sendResponse(ok, outE); }, + [responseHandler](std::exception_ptr ex) { responseHandler->sendException(ex); }, + responseHandler->current()); } catch (...) { - incomingPtr->failed(std::current_exception()); + responseHandler->sendException(std::current_exception()); } - return false; } diff --git a/cpp/src/Ice/ObjectAdapterI.cpp b/cpp/src/Ice/ObjectAdapterI.cpp index 3117e4fd6f3..a0ba163d7ab 100644 --- a/cpp/src/Ice/ObjectAdapterI.cpp +++ b/cpp/src/Ice/ObjectAdapterI.cpp @@ -29,6 +29,8 @@ #include #include "Ice/ProxyFunctions.h" #include "CheckIdentity.h" +#include "LoggerMiddleware.h" +#include "ObserverMiddleware.h" #ifdef _WIN32 # include @@ -513,6 +515,17 @@ Ice::ObjectAdapterI::findServantLocator(const string& prefix) const return _servantManager->findServantLocator(prefix); } +ObjectPtr +Ice::ObjectAdapterI::dispatcher() const noexcept +{ + // _dispatcher is immutable, so no need to lock _mutex. There is no need to clear _dispatcher during destroy + // because _dispatcher does not hold onto this object adapter directly. It can hold onto a communicator that holds + // onto this object adapter, but the communicator will release this refcount when it is destroyed or when the + // object adapter is destroyed. + + return _dispatcher; +} + ObjectPrx Ice::ObjectAdapterI::createProxy(const Identity& ident) const { @@ -845,7 +858,7 @@ Ice::ObjectAdapterI::setAdapterOnConnection(const Ice::ConnectionIPtr& connectio { lock_guard lock(_mutex); checkForDeactivation(); - connection->setAdapterAndServantManager(shared_from_this(), _servantManager); + connection->setAdapterFromAdapter(shared_from_this()); } // @@ -863,12 +876,30 @@ Ice::ObjectAdapterI::ObjectAdapterI( _instance(instance), _communicator(communicator), _objectAdapterFactory(objectAdapterFactory), - _servantManager(new ServantManager(instance, name)), + _servantManager(make_shared(instance, name)), _name(name), _directCount(0), _noConfig(noConfig), _messageSizeMax(0) { + _dispatcher = _servantManager; + + const auto& observer = _instance->initializationData().observer; + if (observer) + { + _dispatcher = make_shared(_dispatcher, observer); + } + + const auto& logger = _instance->initializationData().logger; + if (logger) + { + int warningLevel = + _instance->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1); + if (warningLevel > 0) + { + _dispatcher = make_shared(_dispatcher, logger, warningLevel, _instance->toStringMode()); + } + } } void diff --git a/cpp/src/Ice/ObjectAdapterI.h b/cpp/src/Ice/ObjectAdapterI.h index 2c1b90e4566..8d6f2c5b741 100644 --- a/cpp/src/Ice/ObjectAdapterI.h +++ b/cpp/src/Ice/ObjectAdapterI.h @@ -62,10 +62,10 @@ namespace Ice FacetMap findAllFacets(const Identity&) const final; std::shared_ptr findByProxy(const ObjectPrx&) const final; std::shared_ptr findDefaultServant(const std::string&) const final; - void addServantLocator(const std::shared_ptr&, const std::string&) final; std::shared_ptr removeServantLocator(const std::string&) final; std::shared_ptr findServantLocator(const std::string&) const final; + ObjectPtr dispatcher() const noexcept final; ObjectPrx createProxy(const Identity&) const final; ObjectPrx createDirectProxy(const Identity&) const final; @@ -137,7 +137,8 @@ namespace Ice IceInternal::ObjectAdapterFactoryPtr _objectAdapterFactory; IceInternal::ThreadPoolPtr _threadPool; IceInternal::ACMConfig _acm; - IceInternal::ServantManagerPtr _servantManager; + const IceInternal::ServantManagerPtr _servantManager; + ObjectPtr _dispatcher; const std::string _name; const std::string _id; const std::string _replicaGroupId; diff --git a/cpp/src/Ice/ObserverMiddleware.cpp b/cpp/src/Ice/ObserverMiddleware.cpp new file mode 100644 index 00000000000..03aa43acc52 --- /dev/null +++ b/cpp/src/Ice/ObserverMiddleware.cpp @@ -0,0 +1,97 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#include "ObserverMiddleware.h" + +using namespace std; +using namespace Ice; +using namespace Ice::Instrumentation; +using namespace IceInternal; + +ObserverMiddleware::ObserverMiddleware(Ice::ObjectPtr next, CommunicatorObserverPtr communicatorObserver) + : _next(std::move(next)), + _communicatorObserver(std::move(communicatorObserver)) +{ + assert(_next); + assert(_communicatorObserver); +} + +void +ObserverMiddleware::dispatch(Ice::IncomingRequest& request, function sendResponse) +{ + auto observerPtr = _communicatorObserver->getDispatchObserver(request.current(), request.size()); + if (observerPtr) + { + observerPtr->attach(); + exception_ptr exception; + + try + { + // We can't move sendResponse as we need it when we catch an exception; so we copy it. + _next->dispatch( + request, + [sendResponse, observerPtr](OutgoingResponse response) + { + switch (response.replyStatus()) + { + case ReplyStatus::Ok: + // don't do anything + break; + case ReplyStatus::UserException: + observerPtr->userException(); + break; + default: + observerPtr->failed(response.exceptionId()); + break; + } + + if (response.current().requestId != 0) + { + observerPtr->reply(response.size()); + } + + observerPtr->detach(); + sendResponse(std::move(response)); + }); + } + catch (const UserException&) + { + observerPtr->userException(); + exception = current_exception(); + } + catch (const Ice::Exception& ex) + { + observerPtr->failed(ex.ice_id()); + exception = current_exception(); + } + catch (const std::exception& ex) + { + observerPtr->failed(ex.what()); + exception = current_exception(); + } + catch (...) + { + observerPtr->failed("unknown"); + exception = current_exception(); + } + + if (exception) + { + OutgoingResponse response = makeOutgoingResponse(exception, request.current()); + if (response.current().requestId != 0) + { + observerPtr->reply(response.size()); + } + observerPtr->detach(); + + // If we get a synchronous exception, sendResponse was not called. + sendResponse(std::move(response)); + } + } + else + { + // Not observing dispatches. + _next->dispatch(request, std::move(sendResponse)); + } +} diff --git a/cpp/src/Ice/ObserverMiddleware.h b/cpp/src/Ice/ObserverMiddleware.h new file mode 100644 index 00000000000..ae6c24eb77b --- /dev/null +++ b/cpp/src/Ice/ObserverMiddleware.h @@ -0,0 +1,27 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#ifndef ICE_OBSERVER_MIDDLEWARE_H +#define ICE_OBSERVER_MIDDLEWARE_H + +#include "Ice/Object.h" +#include "Ice/Instrumentation.h" + +namespace IceInternal +{ + // A middleware that observes dispatches. + class ObserverMiddleware final : public Ice::Object + { + public: + ObserverMiddleware(Ice::ObjectPtr next, Ice::Instrumentation::CommunicatorObserverPtr communicatorObserver); + + void dispatch(Ice::IncomingRequest&, std::function) final; + + private: + Ice::ObjectPtr _next; + Ice::Instrumentation::CommunicatorObserverPtr _communicatorObserver; + }; +} + +#endif diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp new file mode 100644 index 00000000000..e57daa395b2 --- /dev/null +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -0,0 +1,314 @@ +// +// Copyright (c) ZeroC, Inc. All rights reserved. +// + +#include "Ice/ObjectAdapter.h" +#include "Ice/OutgoingResponse.h" + +using namespace std; +using namespace Ice; +using namespace IceInternal; + +namespace IceUtilInternal +{ + extern bool printStackTraces; +} + +namespace +{ + // The "core" implementation of makeOutgoingResponse for exceptions. Note that it can throw an exception. + OutgoingResponse makeOutgoingResponseCore(std::exception_ptr exc, const Current& current) + { + OutputStream ostr(current.adapter->getCommunicator(), Ice::currentProtocolEncoding); + + if (current.requestId != 0) + { + ostr.writeBlob(replyHdr, sizeof(replyHdr)); + ostr.write(current.requestId); + } + ReplyStatus replyStatus; + string exceptionId; + string errorMessage; + + try + { + rethrow_exception(exc); + } + catch (RequestFailedException& rfe) + { + exceptionId = rfe.ice_id(); + errorMessage = rfe.what(); + + if (dynamic_cast(&rfe)) + { + replyStatus = ReplyStatus::ObjectNotExist; + } + else if (dynamic_cast(&rfe)) + { + replyStatus = ReplyStatus::FacetNotExist; + } + else if (dynamic_cast(&rfe)) + { + replyStatus = ReplyStatus::OperationNotExist; + } + else + { + assert(false); + } + + if (rfe.id.name.empty()) + { + rfe.id = current.id; + } + + if (rfe.facet.empty() && !current.facet.empty()) + { + rfe.facet = current.facet; + } + + if (rfe.operation.empty() && !current.operation.empty()) + { + rfe.operation = current.operation; + } + + if (current.requestId != 0) + { + ostr.write(static_cast(replyStatus)); + ostr.write(rfe.id); + + if (rfe.facet.empty()) + { + ostr.write(static_cast(nullptr), static_cast(nullptr)); + } + else + { + ostr.write(&rfe.facet, &rfe.facet + 1); + } + + ostr.write(rfe.operation, false); + } + } + catch (const UserException& ex) + { + exceptionId = ex.ice_id(); + errorMessage = ex.what(); + + replyStatus = ReplyStatus::UserException; + + if (current.requestId != 0) + { + ostr.write(static_cast(replyStatus)); + ostr.startEncapsulation(current.encoding, FormatType::SlicedFormat); + ostr.write(ex); + ostr.endEncapsulation(); + } + } + catch (const UnknownLocalException& ex) + { + exceptionId = ex.ice_id(); + replyStatus = ReplyStatus::UnknownLocalException; + errorMessage = ex.unknown; + } + catch (const UnknownUserException& ex) + { + exceptionId = ex.ice_id(); + replyStatus = ReplyStatus::UnknownUserException; + errorMessage = ex.unknown; + } + catch (const UnknownException& ex) + { + exceptionId = ex.ice_id(); + replyStatus = ReplyStatus::UnknownException; + errorMessage = ex.unknown; + } + catch (const LocalException& ex) + { + exceptionId = ex.ice_id(); + replyStatus = ReplyStatus::UnknownLocalException; + ostringstream str; + str << ex; + if (IceUtilInternal::printStackTraces) + { + str << '\n' << ex.ice_stackTrace(); + } + errorMessage = str.str(); + } + catch (const Exception& ex) + { + exceptionId = ex.ice_id(); + replyStatus = ReplyStatus::UnknownException; + ostringstream str; + str << ex; + if (IceUtilInternal::printStackTraces) + { + str << '\n' << ex.ice_stackTrace(); + } + errorMessage = str.str(); + } + catch (const std::exception& ex) + { + replyStatus = ReplyStatus::UnknownException; + exceptionId = ex.what(); + ostringstream str; + str << "c++ exception: " << exceptionId; + errorMessage = str.str(); + } + catch (...) + { + replyStatus = ReplyStatus::UnknownException; + exceptionId = "unknown"; + errorMessage = "c++ exception: unknown"; + } + + if ((current.requestId != 0) && + (replyStatus == ReplyStatus::UnknownUserException || replyStatus == ReplyStatus::UnknownLocalException || + replyStatus == ReplyStatus::UnknownException)) + { + ostr.write(static_cast(replyStatus)); + ostr.write(errorMessage); + } + + return OutgoingResponse{replyStatus, std::move(exceptionId), std::move(errorMessage), ostr, current}; + } +} // anonymous namespace + +OutgoingResponse::OutgoingResponse( + ReplyStatus replyStatus, + string exceptionId, + string errorMessage, + OutputStream& outputStream, + const Current& current) noexcept + : _current(current), + _exceptionId(std::move(exceptionId)), + _errorMessage(std::move(errorMessage)), + _replyStatus(replyStatus) +{ + _outputStream.swap(outputStream); +} + +OutgoingResponse::OutgoingResponse(OutgoingResponse&& other) noexcept + : _current(std::move(other._current)), + _exceptionId(std::move(other._exceptionId)), + _errorMessage(std::move(other._errorMessage)), + _replyStatus(other._replyStatus) +{ + _outputStream.swap(other._outputStream); +} + +OutgoingResponse& +OutgoingResponse::operator=(OutgoingResponse&& other) noexcept +{ + _current = std::move(other._current); + _replyStatus = other._replyStatus; + _exceptionId = std::move(other._exceptionId); + _errorMessage = std::move(other._errorMessage); + _outputStream.swap(other._outputStream); + return *this; +} + +int32_t +OutgoingResponse::size() const noexcept +{ + return static_cast(_outputStream.b.size() - headerSize - 4); +} + +OutgoingResponse +Ice::makeOutgoingResponse( + std::function marshal, + const Current& current, + FormatType format) noexcept +{ + assert(marshal); + OutputStream ostr(current.adapter->getCommunicator(), Ice::currentProtocolEncoding); + if (current.requestId != 0) + { + try + { + ostr.writeBlob(replyHdr, sizeof(replyHdr)); + ostr.write(current.requestId); + ostr.write(static_cast(ReplyStatus::Ok)); + ostr.startEncapsulation(current.encoding, format); + marshal(&ostr); + ostr.endEncapsulation(); + return OutgoingResponse{ostr, current}; + } + catch (...) + { + return makeOutgoingResponse(current_exception(), current); + } + } + else + { + // A oneway request cannot have a return or out params. + assert(0); + } + return OutgoingResponse{ostr, current}; +} + +OutgoingResponse +Ice::makeEmptyOutgoingResponse(const Current& current) noexcept +{ + OutputStream ostr(current.adapter->getCommunicator(), Ice::currentProtocolEncoding); + if (current.requestId != 0) + { + try + { + ostr.writeBlob(replyHdr, sizeof(replyHdr)); + ostr.write(current.requestId); + ostr.write(static_cast(ReplyStatus::Ok)); + ostr.writeEmptyEncapsulation(current.encoding); + } + catch (...) + { + return makeOutgoingResponse(current_exception(), current); + } + } + return OutgoingResponse{ostr, current}; +} + +OutgoingResponse +Ice::makeOutgoingResponse( + bool ok, + const pair& encapsulation, + const Current& current) noexcept +{ + OutputStream ostr(current.adapter->getCommunicator(), Ice::currentProtocolEncoding); + if (current.requestId != 0) + { + try + { + ostr.writeBlob(replyHdr, sizeof(replyHdr)); + ostr.write(current.requestId); + ostr.write(static_cast(ok ? ReplyStatus::Ok : ReplyStatus::UserException)); + ptrdiff_t size = encapsulation.second - encapsulation.first; + assert(size >= 0); + if (size > 0) + { + ostr.writeEncapsulation(encapsulation.first, static_cast(size)); + } + else + { + ostr.writeEmptyEncapsulation(current.encoding); + } + } + catch (...) + { + return makeOutgoingResponse(current_exception(), current); + } + } + return OutgoingResponse{ostr, current}; +} + +OutgoingResponse +Ice::makeOutgoingResponse(std::exception_ptr exc, const Current& current) noexcept +{ + try + { + return makeOutgoingResponseCore(exc, current); + } + catch (...) + { + // This could throw again, but then it's either a bug or we can't allocate anything. + return makeOutgoingResponseCore(current_exception(), current); + } +} diff --git a/cpp/src/Ice/ResponseHandler.h b/cpp/src/Ice/ResponseHandler.h deleted file mode 100644 index 168f64c7017..00000000000 --- a/cpp/src/Ice/ResponseHandler.h +++ /dev/null @@ -1,29 +0,0 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// - -#ifndef ICE_RESPONSE_HANDLER_H -#define ICE_RESPONSE_HANDLER_H - -#include -#include -#include -#include - -namespace Ice -{ - class OutputStream; -} - -namespace IceInternal -{ - class ResponseHandler : public EnableSharedFromThis - { - public: - virtual void sendResponse(std::int32_t, Ice::OutputStream*, std::uint8_t) = 0; - virtual void sendNoResponse() = 0; - virtual void invokeException(std::int32_t, std::exception_ptr, int) = 0; - }; -} - -#endif diff --git a/cpp/src/Ice/ServantManager.cpp b/cpp/src/Ice/ServantManager.cpp index 111a2878171..b83b3e882bf 100644 --- a/cpp/src/Ice/ServantManager.cpp +++ b/cpp/src/Ice/ServantManager.cpp @@ -480,3 +480,81 @@ IceInternal::ServantManager::destroy() locatorMap.clear(); defaultServantMap.clear(); } + +void +ServantManager::dispatch(IncomingRequest& request, function sendResponse) +{ + const Current& current = request.current(); + ObjectPtr servant = findServant(current.id, current.facet); + + if (servant) + { + // the simple, common path. + servant->dispatch(request, std::move(sendResponse)); + return; + } + + // Else, check servant locators + ServantLocatorPtr locator = findServantLocator(current.id.category); + if (!locator && !current.id.category.empty()) + { + locator = findServantLocator(""); + } + + if (locator) + { + shared_ptr cookie; + try + { + servant = locator->locate(current, cookie); + } + catch (...) + { + // Skip the encapsulation. This allows the next batch requests in the same InputStream to proceed. + request.inputStream().skipEncapsulation(); + throw; + } + + if (servant) + { + try + { + servant->dispatch( + request, + [sendResponse = std::move(sendResponse), locator, servant, cookie](OutgoingResponse response) + { + try + { + locator->finished(response.current(), servant, cookie); + } + catch (...) + { + sendResponse(makeOutgoingResponse(std::current_exception(), response.current())); + return; // done + } + sendResponse(std::move(response)); + }); + return; // done + } + catch (...) + { + // When we catch an exception, the dispatch logic guarantees sendResponse was not called. + locator->finished(current, servant, cookie); + throw; + } + } + } + + assert(!servant); + + // Skip the encapsulation. This allows the next batch requests in the same InputStream to proceed. + request.inputStream().skipEncapsulation(); + if (hasServant(current.id)) + { + throw FacetNotExistException{__FILE__, __LINE__}; + } + else + { + throw ObjectNotExistException{__FILE__, __LINE__}; + } +} diff --git a/cpp/src/Ice/ServantManager.h b/cpp/src/Ice/ServantManager.h index 882f076c1a3..3b8456c4bc1 100644 --- a/cpp/src/Ice/ServantManager.h +++ b/cpp/src/Ice/ServantManager.h @@ -5,10 +5,11 @@ #ifndef ICE_SERVANT_MANAGER_H #define ICE_SERVANT_MANAGER_H -#include -#include -#include -#include +#include "Ice/ServantManagerF.h" +#include "Ice/InstanceF.h" +#include "Ice/Identity.h" +#include "Ice/FacetMap.h" +#include "Ice/Object.h" #include @@ -20,7 +21,8 @@ namespace Ice namespace IceInternal { - class ServantManager + // A dispatcher that manages servants and servant locators for an object adapter. + class ServantManager final : public Ice::Object { public: ServantManager(const InstancePtr&, const std::string&); @@ -39,6 +41,8 @@ namespace IceInternal std::shared_ptr removeServantLocator(const std::string&); std::shared_ptr findServantLocator(const std::string&) const; + void dispatch(Ice::IncomingRequest&, std::function) final; + private: void destroy(); friend class Ice::ObjectAdapterI; diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp index 30fcbb24268..59ba192584d 100644 --- a/cpp/src/slice2cpp/Gen.cpp +++ b/cpp/src/slice2cpp/Gen.cpp @@ -800,8 +800,8 @@ Slice::Gen::generate(const UnitPtr& p) { // For simplicity, we include these extra headers all the time. - C << "\n#include "; // for proxies - C << "\n#include "; // for dispatches + C << "\n#include "; // for proxies + C << "\n#include "; // for async dispatches } // @@ -1835,8 +1835,8 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p) C << nl << "return ::IceInternal::makeLambdaOutgoing<" << lambdaT << ">" << spar; - C << "std::move(" + (lambdaOutParams.size() > 1 ? string("responseCb") : "response") + ")" << "std::move(ex)" - << "std::move(sent)" << "this"; + C << "::std::move(" + (lambdaOutParams.size() > 1 ? string("responseCb") : "response") + ")" << "::std::move(ex)" + << "::std::move(sent)" << "this"; C << string("&" + getUnqualified(scoped, interfaceScope.substr(2)) + lambdaImplPrefix + name); C << inParamsImpl; C << "context" << epar << ";"; @@ -2756,13 +2756,17 @@ Slice::Gen::InterfaceVisitor::visitInterfaceDefEnd(const InterfaceDefPtr& p) H << sp; H << nl << "/// \\cond INTERNAL"; - H << nl << "virtual bool _iceDispatch(::IceInternal::Incoming&) override;"; + H << nl + << "void dispatch(::Ice::IncomingRequest&, ::std::function) " + "override;"; H << nl << "/// \\endcond"; C << sp; C << nl << "/// \\cond INTERNAL"; - C << nl << "bool"; - C << nl << scoped.substr(2) << "::_iceDispatch(::IceInternal::Incoming& incoming)"; + C << nl << "void"; + C << nl << scoped.substr(2) + << "::dispatch(::Ice::IncomingRequest& request, ::std::function " + "sendResponse)"; C << sb; C << sp; @@ -2776,13 +2780,15 @@ Slice::Gen::InterfaceVisitor::visitInterfaceDefEnd(const InterfaceDefPtr& p) C << ";"; C << sp; - C << nl << "const ::Ice::Current& current = incoming.current();"; + C << nl << "const ::Ice::Current& current = request.current();"; C << nl << "::std::pair r = " << "::std::equal_range(allOperations, allOperations" << " + " << allOpNames.size() << ", current.operation);"; C << nl << "if(r.first == r.second)"; C << sb; - C << nl << "throw " << getUnqualified("::Ice::OperationNotExistException", scope) - << "(__FILE__, __LINE__, current.id, current.facet, current.operation);"; + C << nl + << "sendResponse(::Ice::makeOutgoingResponse(::std::make_exception_ptr(::Ice::OperationNotExistException(__" + "FILE__, __LINE__)), current));"; + C << nl << "return;"; C << eb; C << sp; C << nl << "switch(r.first - allOperations)"; @@ -2792,14 +2798,16 @@ Slice::Gen::InterfaceVisitor::visitInterfaceDefEnd(const InterfaceDefPtr& p) { C << nl << "case " << i++ << ':'; C << sb; - C << nl << "return _iceD_" << *q << "(incoming);"; + C << nl << "_iceD_" << *q << "(request, ::std::move(sendResponse));"; + C << nl << "break;"; C << eb; } C << nl << "default:"; C << sb; C << nl << "assert(false);"; - C << nl << "throw " << getUnqualified("::Ice::OperationNotExistException", scope) - << "(__FILE__, __LINE__, current.id, current.facet, current.operation);"; + C << nl + << "sendResponse(::Ice::makeOutgoingResponse(::std::make_exception_ptr(::Ice::OperationNotExistException(__" + "FILE__, __LINE__)), current));"; C << eb; C << eb; C << eb; @@ -2911,23 +2919,26 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) string resultName = marshaledResultStructName(name); params.push_back("::std::function " + responsecbParam); args.push_back( - "[incomingPtr](" + resultName + - " marshaledResult) { incomingPtr->response(::std::move(marshaledResult)); }"); + "[responseHandler](" + resultName + + " marshaledResult) { responseHandler->sendResponse(::std::move(marshaledResult)); }"); } else { params.push_back("::std::function " + responsecbParam); - args.push_back(ret || !outParams.empty() ? "responseCB" : "[incomingPtr] { incomingPtr->response(); }"); + args.push_back( + ret || !outParams.empty() ? "::std::move(responseCb)" + : "[responseHandler] { responseHandler->sendEmptyResponse(); }"); } params.push_back("::std::function " + excbParam); - args.push_back("[incomingPtr](std::exception_ptr ex) { incomingPtr->completed(ex); }"); + args.push_back("[responseHandler](std::exception_ptr ex) { " + "responseHandler->sendException(ex); }"); params.push_back(currentDecl); - args.push_back("incomingPtr->current()"); + args.push_back("responseHandler->current()"); } else { params.push_back(currentDecl); - args.push_back("incoming.current()"); + args.push_back("request.current()"); } if (p->hasMarshaledResult()) @@ -3013,43 +3024,43 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) } H << nl << deprecateSymbol << "virtual " << retS << ' ' << opName << spar << params << epar << isConst << " = 0;"; H << nl << "/// \\cond INTERNAL"; - H << nl << "bool _iceD_" << name << "(::IceInternal::Incoming&)" << isConst << ';'; + H << nl << "void _iceD_" << name << "(::Ice::IncomingRequest&, ::std::function)" + << isConst << ';'; H << nl << "/// \\endcond"; C << sp; C << nl << "/// \\cond INTERNAL"; - C << nl << "bool"; + C << nl << "void"; C << nl << scope.substr(2); - C << "_iceD_" << name << "(::IceInternal::Incoming& incoming)" << isConst; + C << "_iceD_" << name + << "(::Ice::IncomingRequest& request, ::std::function sendResponse)" << isConst; + C << sb; C << nl << "_iceCheckMode(" << getUnqualified(operationModeToString(p->mode()), interfaceScope) - << ", incoming.current().mode);"; + << ", request.current().mode);"; if (!inParams.empty()) { - C << nl << "auto istr = incoming.startReadParams();"; + C << nl << "auto istr = &request.inputStream();"; + C << nl << "istr->startEncapsulation();"; writeAllocateCode(C, inParams, nullptr, interfaceScope, _useWstring | TypeContext::UnmarshalParamZeroCopy); writeUnmarshalCode(C, inParams, nullptr); if (p->sendsClasses(false)) { C << nl << "istr->readPendingValues();"; } - C << nl << "incoming.endReadParams();"; + C << nl << "istr->endEncapsulation();"; } else { - C << nl << "incoming.readEmptyParams();"; - } - if (p->format() != DefaultFormat) - { - C << nl << "incoming.setFormat(" << opFormatTypeToString(p) << ");"; + C << nl << "request.inputStream().skipEmptyEncapsulation();"; } if (!amd) { if (p->hasMarshaledResult()) { - C << nl << "incoming.setMarshaledResult("; + C << nl << "sendResponse(::Ice::OutgoingResponse{"; } else { @@ -3067,43 +3078,63 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) C << "this->" << opName << spar << args << epar; if (p->hasMarshaledResult()) { - C << ");"; + C << ".outputStream(), request.current()});"; } else { C << ";"; if (ret || !outParams.empty()) { - C << nl << "auto ostr = incoming.startWriteParams();"; + C << nl << "sendResponse(::Ice::makeOutgoingResponse([&](::Ice::OutputStream* ostr)"; + C.inc(); + C << sb; writeMarshalCode(C, outParams, p); if (p->returnsClasses(false)) { C << nl << "ostr->writePendingValues();"; } - C << nl << "incoming.endWriteParams();"; + C << eb << ","; + C << nl << "request.current()"; + if (p->format() != DefaultFormat) + { + C << ","; + C << nl << opFormatTypeToString(p); + } + C << "));"; + C.dec(); } else { - C << nl << "incoming.writeEmptyParams();"; + C << nl << "sendResponse(::Ice::makeEmptyOutgoingResponse(request.current()));"; } } - C << nl << "return true;"; } else { - C << nl << "auto incomingPtr = ::std::make_shared<::IceInternal::Incoming>(::std::move(incoming));"; + C << nl + << "auto responseHandler = " + "::std::make_shared<::IceInternal::AsyncResponseHandler>(::std::move(sendResponse), request.current());"; if (!p->hasMarshaledResult() && (ret || !outParams.empty())) { - C << nl << "auto responseCB = [incomingPtr]" << spar << responseParamsDecl << epar; + C << nl << "auto responseCb = [responseHandler]" << spar << responseParamsDecl << epar; + C << sb; + C << nl << "responseHandler->sendResponse("; + C.inc(); + C << nl << "[&](::Ice::OutputStream* ostr)"; C << sb; - C << nl << "auto ostr = incomingPtr->startWriteParams();"; writeMarshalCode(C, outParams, p); if (p->returnsClasses(false)) { C << nl << "ostr->writePendingValues();"; } - C << nl << "incomingPtr->endWriteParams();"; - C << nl << "incomingPtr->completed();"; + C << eb; + if (p->format() != DefaultFormat) + { + C << ","; + C << nl << opFormatTypeToString(p); + } + C << ");"; + C.dec(); C << eb << ';'; } C << nl << "try"; @@ -3112,9 +3143,8 @@ Slice::Gen::InterfaceVisitor::visitOperation(const OperationPtr& p) C << eb; C << nl << "catch (...)"; C << sb; - C << nl << "incomingPtr->failed(::std::current_exception());"; + C << nl << "responseHandler->sendException(::std::current_exception());"; C << eb; - C << nl << "return false;"; } C << eb; C << nl << "/// \\endcond"; From f66a15e7de14e00f784c6821645a76c22930aab2 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 17:43:08 -0400 Subject: [PATCH 02/11] Windows build fixes --- cpp/src/Ice/msbuild/ice/ice.vcxproj | 5 ++++- cpp/src/Ice/msbuild/ice/ice.vcxproj.filters | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cpp/src/Ice/msbuild/ice/ice.vcxproj b/cpp/src/Ice/msbuild/ice/ice.vcxproj index 51da089657f..6c7d41f955f 100644 --- a/cpp/src/Ice/msbuild/ice/ice.vcxproj +++ b/cpp/src/Ice/msbuild/ice/ice.vcxproj @@ -527,7 +527,10 @@ - + + + + diff --git a/cpp/src/Ice/msbuild/ice/ice.vcxproj.filters b/cpp/src/Ice/msbuild/ice/ice.vcxproj.filters index 6e0ed6529c1..4285d50b60a 100644 --- a/cpp/src/Ice/msbuild/ice/ice.vcxproj.filters +++ b/cpp/src/Ice/msbuild/ice/ice.vcxproj.filters @@ -315,7 +315,16 @@ Source Files - + + Source Files + + + Source Files + + + Source Files + + Source Files From 6d7c7af62d159ae8fb2bbbe0a62eecac2c948c11 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 17:57:03 -0400 Subject: [PATCH 03/11] Fix gcc build failure --- cpp/src/Ice/OutgoingResponse.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp index e57daa395b2..8e2f25a1f23 100644 --- a/cpp/src/Ice/OutgoingResponse.cpp +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -54,6 +54,8 @@ namespace else { assert(false); + // Need to set replyStatus otherwise the compiler complains about uninitialized variable. + replyStatus = ReplyStatus::ObjectNotExist; } if (rfe.id.name.empty()) From 281d5d4169f55ceac4fc5c3f8b5ae99a9a2378a2 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 18:15:04 -0400 Subject: [PATCH 04/11] Remove ResponseSentException --- csharp/src/Ice/LocalException.cs | 39 ------------------- .../com/zeroc/Ice/ResponseSentException.java | 39 ------------------- js/src/Ice/LocalException.d.ts | 7 ---- js/src/Ice/LocalException.js | 20 ---------- matlab/lib/+Ice/ResponseSentException.m | 23 ----------- php/lib/IceLocal/LocalException.php | 23 ----------- python/python/Ice/LocalException_local.py | 26 ------------- ruby/ruby/IceLocal/LocalException.rb | 13 ------- swift/Rakefile | 7 +--- swift/src/Ice/LocalException.swift | 17 -------- swift/src/Ice/LocalExceptionDescription.swift | 10 ----- swift/src/Ice/LocalExceptionFactory.swift | 4 -- swift/src/IceImpl/Convert.mm | 4 -- swift/src/IceImpl/Exception.h | 1 - 14 files changed, 1 insertion(+), 232 deletions(-) delete mode 100644 java/src/Ice/src/main/java/com/zeroc/Ice/ResponseSentException.java delete mode 100644 matlab/lib/+Ice/ResponseSentException.m diff --git a/csharp/src/Ice/LocalException.cs b/csharp/src/Ice/LocalException.cs index 432839c0bc7..61eb781ec0d 100644 --- a/csharp/src/Ice/LocalException.cs +++ b/csharp/src/Ice/LocalException.cs @@ -3862,43 +3862,4 @@ public override string ice_id() return "::Ice::FixedProxyException"; } } - - /// - /// Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - /// - - [global::System.Runtime.InteropServices.ComVisible(false)] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1032")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1707")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1709")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1710")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1715")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1716")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1720")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1722")] - [global::System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1724")] - public partial class ResponseSentException : LocalException - { - #region Constructors - - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("slice2cs", "3.7.10")] - public ResponseSentException() - { - } - - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("slice2cs", "3.7.10")] - public ResponseSentException(global::System.Exception ex) : base(ex) - { - } - - #endregion - - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("slice2cs", "3.7.10")] - public override string ice_id() - { - return "::Ice::ResponseSentException"; - } - } } diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/ResponseSentException.java b/java/src/Ice/src/main/java/com/zeroc/Ice/ResponseSentException.java deleted file mode 100644 index 006ff27b07f..00000000000 --- a/java/src/Ice/src/main/java/com/zeroc/Ice/ResponseSentException.java +++ /dev/null @@ -1,39 +0,0 @@ -// -// Copyright (c) ZeroC, Inc. All rights reserved. -// -// -// Ice version 3.7.10 -// -// -// -// Generated from file `LocalException.ice' -// -// Warning: do not edit this file. -// -// -// - -package com.zeroc.Ice; - -/** - * Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - **/ -public class ResponseSentException extends LocalException -{ - public ResponseSentException() - { - } - - public ResponseSentException(Throwable cause) - { - super(cause); - } - - public String ice_id() - { - return "::Ice::ResponseSentException"; - } - - /** @hidden */ - public static final long serialVersionUID = -8662310375309760496L; -} diff --git a/js/src/Ice/LocalException.d.ts b/js/src/Ice/LocalException.d.ts index 7cf9dcf428a..36dd3d5567a 100644 --- a/js/src/Ice/LocalException.d.ts +++ b/js/src/Ice/LocalException.d.ts @@ -888,11 +888,4 @@ export namespace Ice class FixedProxyException extends LocalException { } - - /** - * Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - */ - class ResponseSentException extends LocalException - { - } } diff --git a/js/src/Ice/LocalException.js b/js/src/Ice/LocalException.js index 62e9f6d1021..ef059509aff 100644 --- a/js/src/Ice/LocalException.js +++ b/js/src/Ice/LocalException.js @@ -1482,26 +1482,6 @@ Ice.FixedProxyException = class extends Ice.LocalException } }; -/** - * Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - **/ -Ice.ResponseSentException = class extends Ice.LocalException -{ - constructor(_cause = "") - { - super(_cause); - } - - static get _parent() - { - return Ice.LocalException; - } - - static get _id() - { - return "::Ice::ResponseSentException"; - } -}; /* slice2js browser-bundle-skip */ exports.Ice = Ice; /* slice2js browser-bundle-skip-end */ diff --git a/matlab/lib/+Ice/ResponseSentException.m b/matlab/lib/+Ice/ResponseSentException.m deleted file mode 100644 index de5568fa1ba..00000000000 --- a/matlab/lib/+Ice/ResponseSentException.m +++ /dev/null @@ -1,23 +0,0 @@ -% ResponseSentException Summary of ResponseSentException -% -% Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - -% Copyright (c) ZeroC, Inc. All rights reserved. -% Generated from LocalException.ice by slice2matlab version 3.7.10 - -classdef ResponseSentException < Ice.LocalException - methods - function obj = ResponseSentException(ice_exid, ice_exmsg) - if nargin == 0 || isempty(ice_exid) - ice_exid = 'Ice:ResponseSentException'; - end - if nargin < 2 || isempty(ice_exmsg) - ice_exmsg = 'Ice.ResponseSentException'; - end - obj = obj@Ice.LocalException(ice_exid, ice_exmsg); - end - function id = ice_id(~) - id = '::Ice::ResponseSentException'; - end - end -end diff --git a/php/lib/IceLocal/LocalException.php b/php/lib/IceLocal/LocalException.php index bbf3c9b2107..2911f3d359c 100644 --- a/php/lib/IceLocal/LocalException.php +++ b/php/lib/IceLocal/LocalException.php @@ -1786,27 +1786,4 @@ public function __toString(): string $Ice__t_FixedProxyException = IcePHP_defineException('::Ice::FixedProxyException', '\\Ice\\FixedProxyException', null, null); } -namespace Ice -{ - global $Ice__t_ResponseSentException; - class ResponseSentException extends \Ice\LocalException - { - public function __construct() - { - } - - public function ice_id() - { - return '::Ice::ResponseSentException'; - } - - public function __toString(): string - { - global $Ice__t_ResponseSentException; - return IcePHP_stringifyException($this, $Ice__t_ResponseSentException); - } - } - - $Ice__t_ResponseSentException = IcePHP_defineException('::Ice::ResponseSentException', '\\Ice\\ResponseSentException', null, null); -} ?> diff --git a/python/python/Ice/LocalException_local.py b/python/python/Ice/LocalException_local.py index be2b4c13d90..33c21dc513d 100644 --- a/python/python/Ice/LocalException_local.py +++ b/python/python/Ice/LocalException_local.py @@ -2140,30 +2140,4 @@ def __str__(self): _M_Ice.FixedProxyException = FixedProxyException del FixedProxyException -if "ResponseSentException" not in _M_Ice.__dict__: - _M_Ice.ResponseSentException = Ice.createTempClass() - - class ResponseSentException(Ice.LocalException): - """ - Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. - """ - - def __init__(self): - pass - - def __str__(self): - return IcePy.stringifyException(self) - - __repr__ = __str__ - - _ice_id = "::Ice::ResponseSentException" - - _M_Ice._t_ResponseSentException = IcePy.defineException( - "::Ice::ResponseSentException", ResponseSentException, (), None, () - ) - ResponseSentException._ice_type = _M_Ice._t_ResponseSentException - - _M_Ice.ResponseSentException = ResponseSentException - del ResponseSentException - # End of module Ice diff --git a/ruby/ruby/IceLocal/LocalException.rb b/ruby/ruby/IceLocal/LocalException.rb index ee2a099746c..a169550dc35 100644 --- a/ruby/ruby/IceLocal/LocalException.rb +++ b/ruby/ruby/IceLocal/LocalException.rb @@ -1012,17 +1012,4 @@ def to_s T_FixedProxyException = ::Ice::__defineException('::Ice::FixedProxyException', FixedProxyException, nil, []) end - - if not defined?(::Ice::ResponseSentException) - class ResponseSentException < Ice::LocalException - def initialize - end - - def to_s - '::Ice::ResponseSentException' - end - end - - T_ResponseSentException = ::Ice::__defineException('::Ice::ResponseSentException', ResponseSentException, nil, []) - end end diff --git a/swift/Rakefile b/swift/Rakefile index 4110a256d89..32c8985c6df 100644 --- a/swift/Rakefile +++ b/swift/Rakefile @@ -153,12 +153,7 @@ def create_platform_targets(project, platform, bindist) end excludes = { - "Ice" => ["Application.cpp", - "AsyncResult.cpp", - "AsyncResult.cpp", - "DLLMain.cpp", - "ResponseHandler.cpp", - "SystemdJournal.cpp", + "Ice" => ["DLLMain.cpp", "OpenSSL*", "SChannel*"] } diff --git a/swift/src/Ice/LocalException.swift b/swift/src/Ice/LocalException.swift index 4f18fd540dc..7620e9a2e66 100644 --- a/swift/src/Ice/LocalException.swift +++ b/swift/src/Ice/LocalException.swift @@ -1554,20 +1554,3 @@ open class FixedProxyException: LocalException { return _FixedProxyExceptionDescription } } - -/// Indicates that the response to a request has already been sent; re-dispatching such a request is not possible. -open class ResponseSentException: LocalException { - /// Returns the Slice type ID of this exception. - /// - /// - returns: `Swift.String` - the Slice type ID of this exception. - override open class func ice_staticId() -> Swift.String { - return "::Ice::ResponseSentException" - } - - /// Returns a string representation of this exception - /// - /// - returns: `Swift.String` - The string representaton of this exception. - override open func ice_print() -> Swift.String { - return _ResponseSentExceptionDescription - } -} diff --git a/swift/src/Ice/LocalExceptionDescription.swift b/swift/src/Ice/LocalExceptionDescription.swift index 98347838e20..60ec1ca74d8 100644 --- a/swift/src/Ice/LocalExceptionDescription.swift +++ b/swift/src/Ice/LocalExceptionDescription.swift @@ -750,13 +750,3 @@ extension FixedProxyException { return s } } - -extension ResponseSentException { - var _ResponseSentExceptionDescription: String { - var s = String() - - s.sep("response sent exception") - - return s - } -} diff --git a/swift/src/Ice/LocalExceptionFactory.swift b/swift/src/Ice/LocalExceptionFactory.swift index 5cfc9493258..1cb1760351f 100644 --- a/swift/src/Ice/LocalExceptionFactory.swift +++ b/swift/src/Ice/LocalExceptionFactory.swift @@ -101,10 +101,6 @@ class ExceptionFactory: ICEExceptionFactory { return FixedProxyException(file: file, line: line) } - static func responseSentException(_ file: String, line: Int) -> Error { - return ResponseSentException(file: file, line: line) - } - static func securityException(_ reason: String, file: String, line: Int) -> Error { return SecurityException(reason: reason, file: file, line: line) } diff --git a/swift/src/IceImpl/Convert.mm b/swift/src/IceImpl/Convert.mm index 925aad5cc2a..c818200d743 100644 --- a/swift/src/IceImpl/Convert.mm +++ b/swift/src/IceImpl/Convert.mm @@ -155,10 +155,6 @@ { return [factory fixedProxyException:toNSString(e.ice_file()) line:e.ice_line()]; } - catch (const Ice::ResponseSentException& e) - { - return [factory responseSentException:toNSString(e.ice_file()) line:e.ice_line()]; - } catch (const Ice::SecurityException& e) { return [factory securityException:toNSString(e.reason) file:toNSString(e.ice_file()) line:e.ice_line()]; diff --git a/swift/src/IceImpl/Exception.h b/swift/src/IceImpl/Exception.h index e503350e959..9cde1346b36 100644 --- a/swift/src/IceImpl/Exception.h +++ b/swift/src/IceImpl/Exception.h @@ -34,7 +34,6 @@ ICEIMPL_API @protocol ICEExceptionFactory + (NSError*)invocationCanceledException:(NSString*)file line:(size_t)line; + (NSError*)featureNotSupportedException:(NSString*)unsupportedFeature file:(NSString*)file line:(size_t)line; + (NSError*)fixedProxyException:(NSString*)file line:(size_t)line; -+ (NSError*)responseSentException:(NSString*)file line:(size_t)line; + (NSError*)securityException:(NSString*)reason file:(NSString*)file line:(size_t)line; + (NSError*)localException:(NSString*)file line:(size_t)line; From 2af6e74cc7deaddf073dd3585e72f3fe55e31aae Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 18:42:28 -0400 Subject: [PATCH 05/11] Small fix to OutgoingResponse::size --- cpp/include/Ice/OutgoingResponse.h | 2 +- cpp/src/Ice/OutgoingResponse.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/Ice/OutgoingResponse.h b/cpp/include/Ice/OutgoingResponse.h index 0e04c8a5b07..a40ae0a358a 100644 --- a/cpp/include/Ice/OutgoingResponse.h +++ b/cpp/include/Ice/OutgoingResponse.h @@ -87,7 +87,7 @@ namespace Ice * @remarks The response only holds onto a reference for this Current object. The caller keeps the Current * object alive until the call to sendResponse completes. */ - const Current& current() const noexcept { return _current; } + const Current& current() const noexcept { return _current.get(); } /** * Return the exception ID of the response. It's empty when replyStatus() is ReplyStatus::Ok. diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp index 8e2f25a1f23..20e8fe149bc 100644 --- a/cpp/src/Ice/OutgoingResponse.cpp +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -211,7 +211,7 @@ OutgoingResponse::operator=(OutgoingResponse&& other) noexcept int32_t OutgoingResponse::size() const noexcept { - return static_cast(_outputStream.b.size() - headerSize - 4); + return _current.get().requestId == 0 ? 0 : static_cast(_outputStream.b.size() - sizeof(replyHdr)); } OutgoingResponse From a296d1450a74a7b0cb40998cd41d920522813d16 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Sun, 10 Mar 2024 18:44:57 -0400 Subject: [PATCH 06/11] Fix copilot hallucination --- cpp/src/Ice/OutgoingResponse.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp index 20e8fe149bc..38aaa7b2a3d 100644 --- a/cpp/src/Ice/OutgoingResponse.cpp +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -211,7 +211,7 @@ OutgoingResponse::operator=(OutgoingResponse&& other) noexcept int32_t OutgoingResponse::size() const noexcept { - return _current.get().requestId == 0 ? 0 : static_cast(_outputStream.b.size() - sizeof(replyHdr)); + return _current.get().requestId == 0 ? 0 : static_cast(_outputStream.b.size() - headerSize - 4); } OutgoingResponse From 64423ebd939c486b920aea3ebac80bd0c0578431 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Mon, 11 Mar 2024 10:58:36 -0400 Subject: [PATCH 07/11] Fix review comments --- cpp/src/Ice/Object.cpp | 4 ++-- cpp/src/Ice/ObserverMiddleware.cpp | 2 +- cpp/src/Ice/OutgoingResponse.cpp | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cpp/src/Ice/Object.cpp b/cpp/src/Ice/Object.cpp index 58be2faf8c2..27108f153c4 100644 --- a/cpp/src/Ice/Object.cpp +++ b/cpp/src/Ice/Object.cpp @@ -275,8 +275,8 @@ Ice::BlobjectArrayAsync::dispatch(IncomingRequest& request, std::function& outE) - { responseHandler->sendResponse(ok, outE); }, + [responseHandler](bool ok, const pair& outEncaps) + { responseHandler->sendResponse(ok, outEncaps); }, [responseHandler](std::exception_ptr ex) { responseHandler->sendException(ex); }, responseHandler->current()); } diff --git a/cpp/src/Ice/ObserverMiddleware.cpp b/cpp/src/Ice/ObserverMiddleware.cpp index 03aa43acc52..7f1b37bb41a 100644 --- a/cpp/src/Ice/ObserverMiddleware.cpp +++ b/cpp/src/Ice/ObserverMiddleware.cpp @@ -20,7 +20,7 @@ ObserverMiddleware::ObserverMiddleware(Ice::ObjectPtr next, CommunicatorObserver void ObserverMiddleware::dispatch(Ice::IncomingRequest& request, function sendResponse) { - auto observerPtr = _communicatorObserver->getDispatchObserver(request.current(), request.size()); + DispatchObserverPtr observerPtr = _communicatorObserver->getDispatchObserver(request.current(), request.size()); if (observerPtr) { observerPtr->attach(); diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp index 38aaa7b2a3d..d84433fa9b5 100644 --- a/cpp/src/Ice/OutgoingResponse.cpp +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -19,6 +19,7 @@ namespace // The "core" implementation of makeOutgoingResponse for exceptions. Note that it can throw an exception. OutgoingResponse makeOutgoingResponseCore(std::exception_ptr exc, const Current& current) { + assert(exc); OutputStream ostr(current.adapter->getCommunicator(), Ice::currentProtocolEncoding); if (current.requestId != 0) @@ -243,8 +244,8 @@ Ice::makeOutgoingResponse( { // A oneway request cannot have a return or out params. assert(0); + return OutgoingResponse{ostr, current}; } - return OutgoingResponse{ostr, current}; } OutgoingResponse From 6d9abc5932184d88a39a569232ec807175954c1f Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Mon, 11 Mar 2024 15:36:01 -0400 Subject: [PATCH 08/11] Work-around for Ruby test crash --- ruby/test/Ice/objects/AllTests.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ruby/test/Ice/objects/AllTests.rb b/ruby/test/Ice/objects/AllTests.rb index 66e5e9e89f7..749a820edf9 100644 --- a/ruby/test/Ice/objects/AllTests.rb +++ b/ruby/test/Ice/objects/AllTests.rb @@ -56,13 +56,15 @@ def test(b) def allTests(helper, communicator) factory = MyValueFactory.new - communicator.getValueFactoryManager().add(factory, '::Test::B') - communicator.getValueFactoryManager().add(factory, '::Test::C') + valueFactoryManager = communicator.getValueFactoryManager() + test(valueFactoryManager != nil) + valueFactoryManager.add(factory, '::Test::B') + valueFactoryManager.add(factory, '::Test::C') #communicator.getValueFactoryManager().add(factory, '::Test::D') - communicator.getValueFactoryManager().add(factory, '::Test::E') - communicator.getValueFactoryManager().add(factory, '::Test::F') - communicator.getValueFactoryManager().add(factory, '::Test::I') - communicator.getValueFactoryManager().add(factory, '::Test::J') + valueFactoryManager.add(factory, '::Test::E') + valueFactoryManager.add(factory, '::Test::F') + valueFactoryManager.add(factory, '::Test::I') + valueFactoryManager.add(factory, '::Test::J') print "testing stringToProxy... " STDOUT.flush From 464269ad44b4ee4017b9d001b097c3baa38a61f2 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Mon, 11 Mar 2024 21:56:51 -0400 Subject: [PATCH 09/11] Add support for inserting dispatcher in other OA --- cpp/include/Ice/ObjectAdapter.h | 1 + cpp/src/Ice/CollocatedRequestHandler.cpp | 2 +- cpp/src/Ice/ConnectionI.cpp | 18 ++++---- cpp/src/Ice/ConnectionI.h | 13 +++--- cpp/src/Ice/ObjectAdapterI.cpp | 56 ++++++++++-------------- cpp/src/Ice/ObjectAdapterI.h | 11 ++++- cpp/test/Ice/metrics/AllTests.cpp | 16 ++++++- cpp/test/Ice/metrics/Collocated.cpp | 6 ++- cpp/test/Ice/metrics/Server.cpp | 7 ++- cpp/test/Ice/metrics/ServerAMD.cpp | 7 ++- 10 files changed, 82 insertions(+), 55 deletions(-) diff --git a/cpp/include/Ice/ObjectAdapter.h b/cpp/include/Ice/ObjectAdapter.h index 4d138c68b86..aa6b99124f7 100644 --- a/cpp/include/Ice/ObjectAdapter.h +++ b/cpp/include/Ice/ObjectAdapter.h @@ -349,6 +349,7 @@ namespace Ice * Get the dispatcher associated with this object adapter. This object dispatches incoming requests to the * servants managed by this object adapter, and takes into account the servant locators. * @return The dispatcher. This shared_ptr is never null. + * @remarks You can add this dispatcher as a servant (including default servant) in another object adapter. */ virtual ObjectPtr dispatcher() const noexcept = 0; diff --git a/cpp/src/Ice/CollocatedRequestHandler.cpp b/cpp/src/Ice/CollocatedRequestHandler.cpp index 03202cf77d9..195d4fdf94d 100644 --- a/cpp/src/Ice/CollocatedRequestHandler.cpp +++ b/cpp/src/Ice/CollocatedRequestHandler.cpp @@ -335,7 +335,7 @@ CollocatedRequestHandler::invokeAll(OutputStream* os, int32_t requestId, int32_t try { - _adapter->dispatcher()->dispatch( + _adapter->dispatchPipeline()->dispatch( request, [self = shared_from_this()](OutgoingResponse response) { self->sendResponse(std::move(response)); }); diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp index e57cb3a13af..021cd781fc4 100644 --- a/cpp/src/Ice/ConnectionI.cpp +++ b/cpp/src/Ice/ConnectionI.cpp @@ -57,7 +57,7 @@ namespace uint8_t compress, int32_t requestId, int32_t invokeNum, - const ObjectAdapterPtr& adapter, + const ObjectAdapterIPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, InputStream& stream) @@ -97,7 +97,7 @@ namespace const uint8_t _compress; const int32_t _requestId; const int32_t _invokeNum; - const ObjectAdapterPtr _adapter; + const ObjectAdapterIPtr _adapter; const OutgoingAsyncBasePtr _outAsync; const HeartbeatCallback _heartbeatCallback; InputStream _stream; @@ -580,7 +580,7 @@ Ice::ConnectionI::waitUntilFinished() // // Clear the OA. See bug 1673 for the details of why this is necessary. // - _adapter = 0; + _adapter = nullptr; } void @@ -1263,7 +1263,7 @@ Ice::ConnectionI::createProxy(const Identity& ident) const } void -Ice::ConnectionI::setAdapterFromAdapter(const ObjectAdapterPtr& adapter) +Ice::ConnectionI::setAdapterFromAdapter(const ObjectAdapterIPtr& adapter) { std::lock_guard lock(_mutex); if (_state <= StateNotValidated || _state >= StateClosing) @@ -1382,7 +1382,7 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) uint8_t compress = 0; int32_t requestId = 0; int32_t invokeNum = 0; - ObjectAdapterPtr adapter; + ObjectAdapterIPtr adapter; OutgoingAsyncBasePtr outAsync; HeartbeatCallback heartbeatCallback; int dispatchCount = 0; @@ -1701,7 +1701,7 @@ ConnectionI::dispatch( uint8_t compress, int32_t requestId, int32_t invokeNum, - const ObjectAdapterPtr& adapter, + const ObjectAdapterIPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, InputStream& stream) @@ -3189,7 +3189,7 @@ Ice::ConnectionI::parseMessage( int32_t& invokeNum, int32_t& requestId, uint8_t& compress, - ObjectAdapterPtr& adapter, + ObjectAdapterIPtr& adapter, OutgoingAsyncBasePtr& outAsync, HeartbeatCallback& heartbeatCallback, int& dispatchCount) @@ -3420,7 +3420,7 @@ Ice::ConnectionI::invokeAll( int32_t invokeNum, int32_t requestId, uint8_t compress, - const ObjectAdapterPtr& adapter) + const ObjectAdapterIPtr& adapter) { // // Note: In contrast to other private or protected methods, this @@ -3441,7 +3441,7 @@ Ice::ConnectionI::invokeAll( { try { - adapter->dispatcher()->dispatch( + adapter->dispatchPipeline()->dispatch( request, [self = shared_from_this(), compress](OutgoingResponse response) { self->sendResponse(std::move(response), compress); }); diff --git a/cpp/src/Ice/ConnectionI.h b/cpp/src/Ice/ConnectionI.h index 99b521ed01a..a91d332492d 100644 --- a/cpp/src/Ice/ConnectionI.h +++ b/cpp/src/Ice/ConnectionI.h @@ -46,6 +46,7 @@ namespace Ice { class LocalException; class ObjectAdapterI; + using ObjectAdapterIPtr = std::shared_ptr; class ConnectionI : public Connection, public IceInternal::EventHandler, public IceInternal::CancellationHandler { @@ -172,12 +173,12 @@ namespace Ice IceInternal::EndpointIPtr endpoint() const; IceInternal::ConnectorPtr connector() const; - virtual void setAdapter(const ObjectAdapterPtr&); // From Connection. + virtual void setAdapter(const ObjectAdapterPtr&); // From Connection. virtual ObjectAdapterPtr getAdapter() const noexcept; // From Connection. virtual EndpointPtr getEndpoint() const noexcept; // From Connection. virtual ObjectPrx createProxy(const Identity& ident) const; // From Connection. - void setAdapterFromAdapter(const ObjectAdapterPtr&); // From ObjectAdapterI. + void setAdapterFromAdapter(const ObjectAdapterIPtr&); // From ObjectAdapterI. // // Operations from EventHandler @@ -208,7 +209,7 @@ namespace Ice std::uint8_t, std::int32_t, std::int32_t, - const ObjectAdapterPtr&, + const ObjectAdapterIPtr&, const IceInternal::OutgoingAsyncBasePtr&, const HeartbeatCallback&, Ice::InputStream&); @@ -278,12 +279,12 @@ namespace Ice std::int32_t&, std::int32_t&, std::uint8_t&, - ObjectAdapterPtr&, + ObjectAdapterIPtr&, IceInternal::OutgoingAsyncBasePtr&, HeartbeatCallback&, int&); - void invokeAll(Ice::InputStream&, std::int32_t, std::int32_t, std::uint8_t, const ObjectAdapterPtr&); + void invokeAll(Ice::InputStream&, std::int32_t, std::int32_t, std::uint8_t, const ObjectAdapterIPtr&); void scheduleTimeout(IceInternal::SocketOperation status); void unscheduleTimeout(IceInternal::SocketOperation status); @@ -307,7 +308,7 @@ namespace Ice mutable Ice::ConnectionInfoPtr _info; - ObjectAdapterPtr _adapter; + ObjectAdapterIPtr _adapter; const bool _hasExecutor; const LoggerPtr _logger; diff --git a/cpp/src/Ice/ObjectAdapterI.cpp b/cpp/src/Ice/ObjectAdapterI.cpp index a0ba163d7ab..60dfa1f5172 100644 --- a/cpp/src/Ice/ObjectAdapterI.cpp +++ b/cpp/src/Ice/ObjectAdapterI.cpp @@ -55,6 +55,27 @@ namespace } inline EndpointIPtr toEndpointI(const EndpointPtr& endp) { return dynamic_pointer_cast(endp); } + + ObjectPtr createDispatchPipeline(Ice::ObjectPtr dispatcher, const InstancePtr& instance) + { + const auto& observer = instance->initializationData().observer; + if (observer) + { + dispatcher = make_shared(std::move(dispatcher), observer); + } + + const auto& logger = instance->initializationData().logger; + if (logger) + { + int warningLevel = + instance->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1); + if (warningLevel > 0) + { + dispatcher = make_shared(std::move(dispatcher), logger, warningLevel, instance->toStringMode()); + } + } + return dispatcher; + } } string @@ -518,12 +539,7 @@ Ice::ObjectAdapterI::findServantLocator(const string& prefix) const ObjectPtr Ice::ObjectAdapterI::dispatcher() const noexcept { - // _dispatcher is immutable, so no need to lock _mutex. There is no need to clear _dispatcher during destroy - // because _dispatcher does not hold onto this object adapter directly. It can hold onto a communicator that holds - // onto this object adapter, but the communicator will release this refcount when it is destroyed or when the - // object adapter is destroyed. - - return _dispatcher; + return _servantManager; } ObjectPrx @@ -835,15 +851,6 @@ Ice::ObjectAdapterI::getThreadPool() const } } -ServantManagerPtr -Ice::ObjectAdapterI::getServantManager() const -{ - // - // No mutex lock necessary, _servantManager is immutable. - // - return _servantManager; -} - IceInternal::ACMConfig Ice::ObjectAdapterI::getACM() const { @@ -877,29 +884,12 @@ Ice::ObjectAdapterI::ObjectAdapterI( _communicator(communicator), _objectAdapterFactory(objectAdapterFactory), _servantManager(make_shared(instance, name)), + _dispatchPipeline(createDispatchPipeline(_servantManager, instance)), _name(name), _directCount(0), _noConfig(noConfig), _messageSizeMax(0) { - _dispatcher = _servantManager; - - const auto& observer = _instance->initializationData().observer; - if (observer) - { - _dispatcher = make_shared(_dispatcher, observer); - } - - const auto& logger = _instance->initializationData().logger; - if (logger) - { - int warningLevel = - _instance->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1); - if (warningLevel > 0) - { - _dispatcher = make_shared(_dispatcher, logger, warningLevel, _instance->toStringMode()); - } - } } void diff --git a/cpp/src/Ice/ObjectAdapterI.h b/cpp/src/Ice/ObjectAdapterI.h index 8d6f2c5b741..c595246b444 100644 --- a/cpp/src/Ice/ObjectAdapterI.h +++ b/cpp/src/Ice/ObjectAdapterI.h @@ -94,11 +94,14 @@ namespace Ice void decDirectCount(); IceInternal::ThreadPoolPtr getThreadPool() const; - IceInternal::ServantManagerPtr getServantManager() const; IceInternal::ACMConfig getACM() const; void setAdapterOnConnection(const Ice::ConnectionIPtr&); size_t messageSizeMax() const { return _messageSizeMax; } + // The dispatch pipeline is the dispatcher plus the logger and observer middleware. They are installed in the + // dispatch pipeline only when the communicator configuration enables them. + const Ice::ObjectPtr& dispatchPipeline() const noexcept { return _dispatchPipeline;} + ObjectAdapterI( const IceInternal::InstancePtr&, const CommunicatorPtr&, @@ -138,7 +141,11 @@ namespace Ice IceInternal::ThreadPoolPtr _threadPool; IceInternal::ACMConfig _acm; const IceInternal::ServantManagerPtr _servantManager; - ObjectPtr _dispatcher; + + // There is no need to clear _dispatchPipeline during destroy because _dispatchPipeline does not hold onto this + // object adapter directly. It can hold onto a communicator that holds onto this object adapter, but the + // communicator will release this refcount when it is destroyed or when the object adapter is destroyed. + const ObjectPtr _dispatchPipeline; // must be declared after _servantManager const std::string _name; const std::string _id; const std::string _replicaGroupId; diff --git a/cpp/test/Ice/metrics/AllTests.cpp b/cpp/test/Ice/metrics/AllTests.cpp index a02a9a0a9c6..fc4bc90f604 100644 --- a/cpp/test/Ice/metrics/AllTests.cpp +++ b/cpp/test/Ice/metrics/AllTests.cpp @@ -371,6 +371,12 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) os << protocol << " -h " << host << " -p " << port; endpoint = os.str(); } + string forwardingEndpoint; + { + ostringstream os; + os << protocol << " -h " << host << " -p " << helper->getTestPort(1); + forwardingEndpoint = os.str(); + } MetricsPrx metrics(communicator, "metrics:" + endpoint); bool collocated = !metrics->ice_getConnection(); @@ -574,7 +580,7 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) map = toMap(clientMetrics->getMetricsView("View", timestamp)["Connection"]); test(map["active"]->current == 1); - ControllerPrx controller(communicator, "controller:" + helper->getTestEndpoint(1)); + ControllerPrx controller(communicator, "controller:" + helper->getTestEndpoint(2)); controller->hold(); map = toMap(clientMetrics->getMetricsView("View", timestamp)["Connection"]); @@ -948,6 +954,14 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) cout << "ok" << endl; + cout << "testing dispatch metrics with forwarding object adapter... " << flush; + MetricsPrx indirectMetrics(communicator, "metrics:" + forwardingEndpoint); + InvokeOp secondOp(indirectMetrics); + + testAttribute(serverMetrics, serverProps, update.get(), "Dispatch", "parent", "ForwardingAdapter", secondOp); + testAttribute(serverMetrics, serverProps, update.get(), "Dispatch", "id", "metrics [op]", secondOp); + cout << "ok" << endl; + cout << "testing invocation metrics... " << flush; props["IceMX.Metrics.View.Map.Invocation.GroupBy"] = "operation"; diff --git a/cpp/test/Ice/metrics/Collocated.cpp b/cpp/test/Ice/metrics/Collocated.cpp index 03c01c2b886..6b996e2ef55 100644 --- a/cpp/test/Ice/metrics/Collocated.cpp +++ b/cpp/test/Ice/metrics/Collocated.cpp @@ -35,7 +35,11 @@ Collocated::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); // adapter->activate(); // Don't activate OA to ensure collocation is used. - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); + communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); + Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); + forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); + + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); // controllerAdapter->activate(); // Don't activate OA to ensure collocation is used. diff --git a/cpp/test/Ice/metrics/Server.cpp b/cpp/test/Ice/metrics/Server.cpp index e774c938622..4c8ff0a488b 100644 --- a/cpp/test/Ice/metrics/Server.cpp +++ b/cpp/test/Ice/metrics/Server.cpp @@ -30,7 +30,12 @@ Server::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); adapter->activate(); - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); + communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); + Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); + forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); + forwardingAdapter->activate(); + + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); controllerAdapter->activate(); diff --git a/cpp/test/Ice/metrics/ServerAMD.cpp b/cpp/test/Ice/metrics/ServerAMD.cpp index d02d3482995..b6508cb7875 100644 --- a/cpp/test/Ice/metrics/ServerAMD.cpp +++ b/cpp/test/Ice/metrics/ServerAMD.cpp @@ -30,7 +30,12 @@ ServerAMD::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); adapter->activate(); - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); + communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); + Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); + forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); + forwardingAdapter->activate(); + + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); controllerAdapter->activate(); From fdfa8e15a75f0da0a6926d8ab5f922afb9e2f852 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Mon, 11 Mar 2024 22:00:11 -0400 Subject: [PATCH 10/11] Revert "Add support for inserting dispatcher in other OA" This reverts commit 464269ad44b4ee4017b9d001b097c3baa38a61f2. --- cpp/include/Ice/ObjectAdapter.h | 1 - cpp/src/Ice/CollocatedRequestHandler.cpp | 2 +- cpp/src/Ice/ConnectionI.cpp | 18 ++++---- cpp/src/Ice/ConnectionI.h | 13 +++--- cpp/src/Ice/ObjectAdapterI.cpp | 56 ++++++++++++++---------- cpp/src/Ice/ObjectAdapterI.h | 11 +---- cpp/test/Ice/metrics/AllTests.cpp | 16 +------ cpp/test/Ice/metrics/Collocated.cpp | 6 +-- cpp/test/Ice/metrics/Server.cpp | 7 +-- cpp/test/Ice/metrics/ServerAMD.cpp | 7 +-- 10 files changed, 55 insertions(+), 82 deletions(-) diff --git a/cpp/include/Ice/ObjectAdapter.h b/cpp/include/Ice/ObjectAdapter.h index aa6b99124f7..4d138c68b86 100644 --- a/cpp/include/Ice/ObjectAdapter.h +++ b/cpp/include/Ice/ObjectAdapter.h @@ -349,7 +349,6 @@ namespace Ice * Get the dispatcher associated with this object adapter. This object dispatches incoming requests to the * servants managed by this object adapter, and takes into account the servant locators. * @return The dispatcher. This shared_ptr is never null. - * @remarks You can add this dispatcher as a servant (including default servant) in another object adapter. */ virtual ObjectPtr dispatcher() const noexcept = 0; diff --git a/cpp/src/Ice/CollocatedRequestHandler.cpp b/cpp/src/Ice/CollocatedRequestHandler.cpp index 195d4fdf94d..03202cf77d9 100644 --- a/cpp/src/Ice/CollocatedRequestHandler.cpp +++ b/cpp/src/Ice/CollocatedRequestHandler.cpp @@ -335,7 +335,7 @@ CollocatedRequestHandler::invokeAll(OutputStream* os, int32_t requestId, int32_t try { - _adapter->dispatchPipeline()->dispatch( + _adapter->dispatcher()->dispatch( request, [self = shared_from_this()](OutgoingResponse response) { self->sendResponse(std::move(response)); }); diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp index 021cd781fc4..e57cb3a13af 100644 --- a/cpp/src/Ice/ConnectionI.cpp +++ b/cpp/src/Ice/ConnectionI.cpp @@ -57,7 +57,7 @@ namespace uint8_t compress, int32_t requestId, int32_t invokeNum, - const ObjectAdapterIPtr& adapter, + const ObjectAdapterPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, InputStream& stream) @@ -97,7 +97,7 @@ namespace const uint8_t _compress; const int32_t _requestId; const int32_t _invokeNum; - const ObjectAdapterIPtr _adapter; + const ObjectAdapterPtr _adapter; const OutgoingAsyncBasePtr _outAsync; const HeartbeatCallback _heartbeatCallback; InputStream _stream; @@ -580,7 +580,7 @@ Ice::ConnectionI::waitUntilFinished() // // Clear the OA. See bug 1673 for the details of why this is necessary. // - _adapter = nullptr; + _adapter = 0; } void @@ -1263,7 +1263,7 @@ Ice::ConnectionI::createProxy(const Identity& ident) const } void -Ice::ConnectionI::setAdapterFromAdapter(const ObjectAdapterIPtr& adapter) +Ice::ConnectionI::setAdapterFromAdapter(const ObjectAdapterPtr& adapter) { std::lock_guard lock(_mutex); if (_state <= StateNotValidated || _state >= StateClosing) @@ -1382,7 +1382,7 @@ Ice::ConnectionI::message(ThreadPoolCurrent& current) uint8_t compress = 0; int32_t requestId = 0; int32_t invokeNum = 0; - ObjectAdapterIPtr adapter; + ObjectAdapterPtr adapter; OutgoingAsyncBasePtr outAsync; HeartbeatCallback heartbeatCallback; int dispatchCount = 0; @@ -1701,7 +1701,7 @@ ConnectionI::dispatch( uint8_t compress, int32_t requestId, int32_t invokeNum, - const ObjectAdapterIPtr& adapter, + const ObjectAdapterPtr& adapter, const OutgoingAsyncBasePtr& outAsync, const HeartbeatCallback& heartbeatCallback, InputStream& stream) @@ -3189,7 +3189,7 @@ Ice::ConnectionI::parseMessage( int32_t& invokeNum, int32_t& requestId, uint8_t& compress, - ObjectAdapterIPtr& adapter, + ObjectAdapterPtr& adapter, OutgoingAsyncBasePtr& outAsync, HeartbeatCallback& heartbeatCallback, int& dispatchCount) @@ -3420,7 +3420,7 @@ Ice::ConnectionI::invokeAll( int32_t invokeNum, int32_t requestId, uint8_t compress, - const ObjectAdapterIPtr& adapter) + const ObjectAdapterPtr& adapter) { // // Note: In contrast to other private or protected methods, this @@ -3441,7 +3441,7 @@ Ice::ConnectionI::invokeAll( { try { - adapter->dispatchPipeline()->dispatch( + adapter->dispatcher()->dispatch( request, [self = shared_from_this(), compress](OutgoingResponse response) { self->sendResponse(std::move(response), compress); }); diff --git a/cpp/src/Ice/ConnectionI.h b/cpp/src/Ice/ConnectionI.h index a91d332492d..99b521ed01a 100644 --- a/cpp/src/Ice/ConnectionI.h +++ b/cpp/src/Ice/ConnectionI.h @@ -46,7 +46,6 @@ namespace Ice { class LocalException; class ObjectAdapterI; - using ObjectAdapterIPtr = std::shared_ptr; class ConnectionI : public Connection, public IceInternal::EventHandler, public IceInternal::CancellationHandler { @@ -173,12 +172,12 @@ namespace Ice IceInternal::EndpointIPtr endpoint() const; IceInternal::ConnectorPtr connector() const; - virtual void setAdapter(const ObjectAdapterPtr&); // From Connection. + virtual void setAdapter(const ObjectAdapterPtr&); // From Connection. virtual ObjectAdapterPtr getAdapter() const noexcept; // From Connection. virtual EndpointPtr getEndpoint() const noexcept; // From Connection. virtual ObjectPrx createProxy(const Identity& ident) const; // From Connection. - void setAdapterFromAdapter(const ObjectAdapterIPtr&); // From ObjectAdapterI. + void setAdapterFromAdapter(const ObjectAdapterPtr&); // From ObjectAdapterI. // // Operations from EventHandler @@ -209,7 +208,7 @@ namespace Ice std::uint8_t, std::int32_t, std::int32_t, - const ObjectAdapterIPtr&, + const ObjectAdapterPtr&, const IceInternal::OutgoingAsyncBasePtr&, const HeartbeatCallback&, Ice::InputStream&); @@ -279,12 +278,12 @@ namespace Ice std::int32_t&, std::int32_t&, std::uint8_t&, - ObjectAdapterIPtr&, + ObjectAdapterPtr&, IceInternal::OutgoingAsyncBasePtr&, HeartbeatCallback&, int&); - void invokeAll(Ice::InputStream&, std::int32_t, std::int32_t, std::uint8_t, const ObjectAdapterIPtr&); + void invokeAll(Ice::InputStream&, std::int32_t, std::int32_t, std::uint8_t, const ObjectAdapterPtr&); void scheduleTimeout(IceInternal::SocketOperation status); void unscheduleTimeout(IceInternal::SocketOperation status); @@ -308,7 +307,7 @@ namespace Ice mutable Ice::ConnectionInfoPtr _info; - ObjectAdapterIPtr _adapter; + ObjectAdapterPtr _adapter; const bool _hasExecutor; const LoggerPtr _logger; diff --git a/cpp/src/Ice/ObjectAdapterI.cpp b/cpp/src/Ice/ObjectAdapterI.cpp index 60dfa1f5172..a0ba163d7ab 100644 --- a/cpp/src/Ice/ObjectAdapterI.cpp +++ b/cpp/src/Ice/ObjectAdapterI.cpp @@ -55,27 +55,6 @@ namespace } inline EndpointIPtr toEndpointI(const EndpointPtr& endp) { return dynamic_pointer_cast(endp); } - - ObjectPtr createDispatchPipeline(Ice::ObjectPtr dispatcher, const InstancePtr& instance) - { - const auto& observer = instance->initializationData().observer; - if (observer) - { - dispatcher = make_shared(std::move(dispatcher), observer); - } - - const auto& logger = instance->initializationData().logger; - if (logger) - { - int warningLevel = - instance->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1); - if (warningLevel > 0) - { - dispatcher = make_shared(std::move(dispatcher), logger, warningLevel, instance->toStringMode()); - } - } - return dispatcher; - } } string @@ -539,7 +518,12 @@ Ice::ObjectAdapterI::findServantLocator(const string& prefix) const ObjectPtr Ice::ObjectAdapterI::dispatcher() const noexcept { - return _servantManager; + // _dispatcher is immutable, so no need to lock _mutex. There is no need to clear _dispatcher during destroy + // because _dispatcher does not hold onto this object adapter directly. It can hold onto a communicator that holds + // onto this object adapter, but the communicator will release this refcount when it is destroyed or when the + // object adapter is destroyed. + + return _dispatcher; } ObjectPrx @@ -851,6 +835,15 @@ Ice::ObjectAdapterI::getThreadPool() const } } +ServantManagerPtr +Ice::ObjectAdapterI::getServantManager() const +{ + // + // No mutex lock necessary, _servantManager is immutable. + // + return _servantManager; +} + IceInternal::ACMConfig Ice::ObjectAdapterI::getACM() const { @@ -884,12 +877,29 @@ Ice::ObjectAdapterI::ObjectAdapterI( _communicator(communicator), _objectAdapterFactory(objectAdapterFactory), _servantManager(make_shared(instance, name)), - _dispatchPipeline(createDispatchPipeline(_servantManager, instance)), _name(name), _directCount(0), _noConfig(noConfig), _messageSizeMax(0) { + _dispatcher = _servantManager; + + const auto& observer = _instance->initializationData().observer; + if (observer) + { + _dispatcher = make_shared(_dispatcher, observer); + } + + const auto& logger = _instance->initializationData().logger; + if (logger) + { + int warningLevel = + _instance->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1); + if (warningLevel > 0) + { + _dispatcher = make_shared(_dispatcher, logger, warningLevel, _instance->toStringMode()); + } + } } void diff --git a/cpp/src/Ice/ObjectAdapterI.h b/cpp/src/Ice/ObjectAdapterI.h index c595246b444..8d6f2c5b741 100644 --- a/cpp/src/Ice/ObjectAdapterI.h +++ b/cpp/src/Ice/ObjectAdapterI.h @@ -94,14 +94,11 @@ namespace Ice void decDirectCount(); IceInternal::ThreadPoolPtr getThreadPool() const; + IceInternal::ServantManagerPtr getServantManager() const; IceInternal::ACMConfig getACM() const; void setAdapterOnConnection(const Ice::ConnectionIPtr&); size_t messageSizeMax() const { return _messageSizeMax; } - // The dispatch pipeline is the dispatcher plus the logger and observer middleware. They are installed in the - // dispatch pipeline only when the communicator configuration enables them. - const Ice::ObjectPtr& dispatchPipeline() const noexcept { return _dispatchPipeline;} - ObjectAdapterI( const IceInternal::InstancePtr&, const CommunicatorPtr&, @@ -141,11 +138,7 @@ namespace Ice IceInternal::ThreadPoolPtr _threadPool; IceInternal::ACMConfig _acm; const IceInternal::ServantManagerPtr _servantManager; - - // There is no need to clear _dispatchPipeline during destroy because _dispatchPipeline does not hold onto this - // object adapter directly. It can hold onto a communicator that holds onto this object adapter, but the - // communicator will release this refcount when it is destroyed or when the object adapter is destroyed. - const ObjectPtr _dispatchPipeline; // must be declared after _servantManager + ObjectPtr _dispatcher; const std::string _name; const std::string _id; const std::string _replicaGroupId; diff --git a/cpp/test/Ice/metrics/AllTests.cpp b/cpp/test/Ice/metrics/AllTests.cpp index fc4bc90f604..a02a9a0a9c6 100644 --- a/cpp/test/Ice/metrics/AllTests.cpp +++ b/cpp/test/Ice/metrics/AllTests.cpp @@ -371,12 +371,6 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) os << protocol << " -h " << host << " -p " << port; endpoint = os.str(); } - string forwardingEndpoint; - { - ostringstream os; - os << protocol << " -h " << host << " -p " << helper->getTestPort(1); - forwardingEndpoint = os.str(); - } MetricsPrx metrics(communicator, "metrics:" + endpoint); bool collocated = !metrics->ice_getConnection(); @@ -580,7 +574,7 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) map = toMap(clientMetrics->getMetricsView("View", timestamp)["Connection"]); test(map["active"]->current == 1); - ControllerPrx controller(communicator, "controller:" + helper->getTestEndpoint(2)); + ControllerPrx controller(communicator, "controller:" + helper->getTestEndpoint(1)); controller->hold(); map = toMap(clientMetrics->getMetricsView("View", timestamp)["Connection"]); @@ -954,14 +948,6 @@ allTests(Test::TestHelper* helper, const CommunicatorObserverIPtr& obsv) cout << "ok" << endl; - cout << "testing dispatch metrics with forwarding object adapter... " << flush; - MetricsPrx indirectMetrics(communicator, "metrics:" + forwardingEndpoint); - InvokeOp secondOp(indirectMetrics); - - testAttribute(serverMetrics, serverProps, update.get(), "Dispatch", "parent", "ForwardingAdapter", secondOp); - testAttribute(serverMetrics, serverProps, update.get(), "Dispatch", "id", "metrics [op]", secondOp); - cout << "ok" << endl; - cout << "testing invocation metrics... " << flush; props["IceMX.Metrics.View.Map.Invocation.GroupBy"] = "operation"; diff --git a/cpp/test/Ice/metrics/Collocated.cpp b/cpp/test/Ice/metrics/Collocated.cpp index 6b996e2ef55..03c01c2b886 100644 --- a/cpp/test/Ice/metrics/Collocated.cpp +++ b/cpp/test/Ice/metrics/Collocated.cpp @@ -35,11 +35,7 @@ Collocated::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); // adapter->activate(); // Don't activate OA to ensure collocation is used. - communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); - Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); - forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); - - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); // controllerAdapter->activate(); // Don't activate OA to ensure collocation is used. diff --git a/cpp/test/Ice/metrics/Server.cpp b/cpp/test/Ice/metrics/Server.cpp index 4c8ff0a488b..e774c938622 100644 --- a/cpp/test/Ice/metrics/Server.cpp +++ b/cpp/test/Ice/metrics/Server.cpp @@ -30,12 +30,7 @@ Server::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); adapter->activate(); - communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); - Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); - forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); - forwardingAdapter->activate(); - - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); controllerAdapter->activate(); diff --git a/cpp/test/Ice/metrics/ServerAMD.cpp b/cpp/test/Ice/metrics/ServerAMD.cpp index b6508cb7875..d02d3482995 100644 --- a/cpp/test/Ice/metrics/ServerAMD.cpp +++ b/cpp/test/Ice/metrics/ServerAMD.cpp @@ -30,12 +30,7 @@ ServerAMD::run(int argc, char** argv) adapter->add(make_shared(), Ice::stringToIdentity("metrics")); adapter->activate(); - communicator->getProperties()->setProperty("ForwardingAdapter.Endpoints", getTestEndpoint(1)); - Ice::ObjectAdapterPtr forwardingAdapter = communicator->createObjectAdapter("ForwardingAdapter"); - forwardingAdapter->addDefaultServant(adapter->dispatcher(), ""); - forwardingAdapter->activate(); - - communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(2)); + communicator->getProperties()->setProperty("ControllerAdapter.Endpoints", getTestEndpoint(1)); Ice::ObjectAdapterPtr controllerAdapter = communicator->createObjectAdapter("ControllerAdapter"); controllerAdapter->add(make_shared(adapter), Ice::stringToIdentity("controller")); controllerAdapter->activate(); From c4618eb29c7206e820210d11d17d285929074721 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 13 Mar 2024 11:01:25 -0400 Subject: [PATCH 11/11] Fix review comments --- cpp/include/Ice/IncomingRequest.h | 14 +++++++++---- cpp/include/Ice/OutgoingResponse.h | 33 +++++++++++++++++++----------- cpp/src/Ice/LoggerMiddleware.cpp | 4 ++-- cpp/src/Ice/OutgoingResponse.cpp | 32 ++++++++++++++--------------- 4 files changed, 49 insertions(+), 34 deletions(-) diff --git a/cpp/include/Ice/IncomingRequest.h b/cpp/include/Ice/IncomingRequest.h index b426eb9e462..5d54ea8f192 100644 --- a/cpp/include/Ice/IncomingRequest.h +++ b/cpp/include/Ice/IncomingRequest.h @@ -29,6 +29,8 @@ namespace Ice * @param adapter The object adapter to set in Current. * @param inputStream The input stream buffer over the incoming Ice protocol request message. The stream is * positioned at the beginning of the request header - the next data to read is the identity of the target. + * @remarks This constructor reads the request header from inputStream. When it completes, the input stream is + * positioned at the beginning of encapsulation carried by the request. */ IncomingRequest( int32_t requestId, @@ -42,22 +44,26 @@ namespace Ice IncomingRequest& operator=(IncomingRequest&&) = delete; /** - * Return the current object of the request. + * Get the current object of the request. + * @return A reference to the current object of the request. */ Current& current() noexcept { return _current; } /** - * Return the current object of the request. + * Get the current object of the request. + * @return A const reference to the current object of the request. */ const Ice::Current& current() const noexcept { return _current; } /** - * Return the input stream buffer of the request. + * Get the input stream buffer of the request. + * @return A reference to the input stream buffer. */ InputStream& inputStream() noexcept { return _inputStream; } /** - * Return the number of bytes in the request. These are all the bytes starting with the identity of the target. + * Get the number of bytes in the request. + * @return The number of bytes in the request. These are all the bytes starting with the identity of the target. */ std::int32_t size() const { return _requestSize; } diff --git a/cpp/include/Ice/OutgoingResponse.h b/cpp/include/Ice/OutgoingResponse.h index a40ae0a358a..ea37c2d4d35 100644 --- a/cpp/include/Ice/OutgoingResponse.h +++ b/cpp/include/Ice/OutgoingResponse.h @@ -43,8 +43,8 @@ namespace Ice /** * Construct an OutgoingResponse object. * @param replyStatus The status of the response. - * @param exceptionId The ID of the exception, when the response carries an error. - * @param errorMessage The error message, when the response carries an error. + * @param exceptionId The ID of the exception, when the response carries an exception. + * @param exceptionMessage The exception message, when the response carries an exception. * @param outputStream The output stream that holds the response. Its underlying buffer is adopted by the new * response. * @param current A reference to the current object of the request. @@ -52,7 +52,7 @@ namespace Ice OutgoingResponse( ReplyStatus replyStatus, std::string exceptionId, - std::string errorMessage, + std::string exceptionMessage, OutputStream& outputStream, const Current& current) noexcept; @@ -83,41 +83,50 @@ namespace Ice OutgoingResponse& operator=(const OutgoingResponse&) = delete; /** - * Return the current object of this response. + * Get the current object of this response. + * @return A const reference to the current object. * @remarks The response only holds onto a reference for this Current object. The caller keeps the Current * object alive until the call to sendResponse completes. */ const Current& current() const noexcept { return _current.get(); } /** - * Return the exception ID of the response. It's empty when replyStatus() is ReplyStatus::Ok. + * Get the exception ID of the response. + * @return The exception ID of the response. It's empty when replyStatus() is ReplyStatus::Ok. Otherwise, this + * ID is the Slice type ID of the exception marshaled into this response if this exception was defined in Slice + * or is derived from Ice::LocalException. For other exceptions, this ID is the value returned by + * std::exception::what(). */ const std::string& exceptionId() const noexcept { return _exceptionId; } /** - * Return the error message of the response. It's empty when replyStatus() is ReplyStatus::Ok. + * Get the exception message of the response. + * @return The exception message. It's empty when replyStatus() is ReplyStatus::Ok. */ - const std::string& errorMessage() const noexcept { return _errorMessage; } + const std::string& exceptionMessage() const noexcept { return _exceptionMessage; } /** - * Return the reply status of the response. + * Get the reply status of the response. + * @return The reply status. */ ReplyStatus replyStatus() const noexcept { return _replyStatus; } /** - * Return the output stream buffer of the response. + * Get the output stream buffer of the response. + * @return A reference to the output stream buffer. */ OutputStream& outputStream() noexcept { return _outputStream; } /** - * Return the number of bytes in the response. + * Get the number of bytes in the response. + * @return The number of bytes in the response. */ std::int32_t size() const noexcept; private: std::reference_wrapper _current; - std::string _exceptionId; // Empty when _replyStatus == Ok. - std::string _errorMessage; // ditto + std::string _exceptionId; + std::string _exceptionMessage; OutputStream _outputStream; ReplyStatus _replyStatus; }; diff --git a/cpp/src/Ice/LoggerMiddleware.cpp b/cpp/src/Ice/LoggerMiddleware.cpp index 81cb8eb4e76..75adcb58b9b 100644 --- a/cpp/src/Ice/LoggerMiddleware.cpp +++ b/cpp/src/Ice/LoggerMiddleware.cpp @@ -40,12 +40,12 @@ LoggerMiddleware::dispatch(Ice::IncomingRequest& request, function_warningLevel > 1) { - self->warning(response.errorMessage(), response.current()); + self->warning(response.exceptionMessage(), response.current()); } break; default: - self->warning(response.errorMessage(), response.current()); + self->warning(response.exceptionMessage(), response.current()); break; } sendResponse(std::move(response)); diff --git a/cpp/src/Ice/OutgoingResponse.cpp b/cpp/src/Ice/OutgoingResponse.cpp index d84433fa9b5..6ff610eb007 100644 --- a/cpp/src/Ice/OutgoingResponse.cpp +++ b/cpp/src/Ice/OutgoingResponse.cpp @@ -29,7 +29,7 @@ namespace } ReplyStatus replyStatus; string exceptionId; - string errorMessage; + string exceptionMessage; try { @@ -38,7 +38,7 @@ namespace catch (RequestFailedException& rfe) { exceptionId = rfe.ice_id(); - errorMessage = rfe.what(); + exceptionMessage = rfe.what(); if (dynamic_cast(&rfe)) { @@ -94,7 +94,7 @@ namespace catch (const UserException& ex) { exceptionId = ex.ice_id(); - errorMessage = ex.what(); + exceptionMessage = ex.what(); replyStatus = ReplyStatus::UserException; @@ -110,19 +110,19 @@ namespace { exceptionId = ex.ice_id(); replyStatus = ReplyStatus::UnknownLocalException; - errorMessage = ex.unknown; + exceptionMessage = ex.unknown; } catch (const UnknownUserException& ex) { exceptionId = ex.ice_id(); replyStatus = ReplyStatus::UnknownUserException; - errorMessage = ex.unknown; + exceptionMessage = ex.unknown; } catch (const UnknownException& ex) { exceptionId = ex.ice_id(); replyStatus = ReplyStatus::UnknownException; - errorMessage = ex.unknown; + exceptionMessage = ex.unknown; } catch (const LocalException& ex) { @@ -134,7 +134,7 @@ namespace { str << '\n' << ex.ice_stackTrace(); } - errorMessage = str.str(); + exceptionMessage = str.str(); } catch (const Exception& ex) { @@ -146,7 +146,7 @@ namespace { str << '\n' << ex.ice_stackTrace(); } - errorMessage = str.str(); + exceptionMessage = str.str(); } catch (const std::exception& ex) { @@ -154,13 +154,13 @@ namespace exceptionId = ex.what(); ostringstream str; str << "c++ exception: " << exceptionId; - errorMessage = str.str(); + exceptionMessage = str.str(); } catch (...) { replyStatus = ReplyStatus::UnknownException; exceptionId = "unknown"; - errorMessage = "c++ exception: unknown"; + exceptionMessage = "c++ exception: unknown"; } if ((current.requestId != 0) && @@ -168,22 +168,22 @@ namespace replyStatus == ReplyStatus::UnknownException)) { ostr.write(static_cast(replyStatus)); - ostr.write(errorMessage); + ostr.write(exceptionMessage); } - return OutgoingResponse{replyStatus, std::move(exceptionId), std::move(errorMessage), ostr, current}; + return OutgoingResponse{replyStatus, std::move(exceptionId), std::move(exceptionMessage), ostr, current}; } } // anonymous namespace OutgoingResponse::OutgoingResponse( ReplyStatus replyStatus, string exceptionId, - string errorMessage, + string exceptionMessage, OutputStream& outputStream, const Current& current) noexcept : _current(current), _exceptionId(std::move(exceptionId)), - _errorMessage(std::move(errorMessage)), + _exceptionMessage(std::move(exceptionMessage)), _replyStatus(replyStatus) { _outputStream.swap(outputStream); @@ -192,7 +192,7 @@ OutgoingResponse::OutgoingResponse( OutgoingResponse::OutgoingResponse(OutgoingResponse&& other) noexcept : _current(std::move(other._current)), _exceptionId(std::move(other._exceptionId)), - _errorMessage(std::move(other._errorMessage)), + _exceptionMessage(std::move(other._exceptionMessage)), _replyStatus(other._replyStatus) { _outputStream.swap(other._outputStream); @@ -204,7 +204,7 @@ OutgoingResponse::operator=(OutgoingResponse&& other) noexcept _current = std::move(other._current); _replyStatus = other._replyStatus; _exceptionId = std::move(other._exceptionId); - _errorMessage = std::move(other._errorMessage); + _exceptionMessage = std::move(other._exceptionMessage); _outputStream.swap(other._outputStream); return *this; }