Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In the SHM-jemalloc session glue layer account for the realistic possibility that when one uses session.lend/borrow_object() to transmit native structures residing in SHM, those operations can fail due to session being down at that time. More or less that means (1) correcting a few log messages and comments and (2) do not "helpfully" assert on things that might not always be true. / Comment and/or doc changes. #53

Merged
merged 2 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ void CLASS_JEM_SRV_SESSION_IMPL::async_accept_log_in
/* Do our extra stuff on top of the base vanilla Server_session_impl. That is Base::session_shm() must
* work (return non-null), as must app_shm() (ditto, cached in this->m_app_shm); plus lend_object()ing
* objects constructed from either must work, and borrow_object()ing from anything the opposing process
* transmits must work. The lend/borrow stuff will work if and only if session_shm() and app_shm() have
* transmits must work. The lend/borrow stuff will work only if session_shm() and app_shm() have
* been registered with the lend/borrow engine, namely shm_session().
* Incidentally Base::shm_session() must also be available (return non-null), in case the user wants to
* access that guy directly.
Expand Down
23 changes: 8 additions & 15 deletions src/ipc/session/detail/shm/arena_lend/jemalloc/session_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,16 +417,19 @@ Error_code CLASS_JEM_SESSION_IMPL::init_shm
* which means, similarly to the above incoming-direction handler do the following.
*
* Same comment and @todo(s) apply as in the handler above. This is a little different, as it's a send error,
* which wouldn't be reported as a channel error; but still we figure it something went this wrong with this
* channel, the session master channel will be hosed too. And even that caveat (again) only matters
* which wouldn't be reported as a channel error; but still we figure if something went this wrong with this
* channel, the session master channel will be hosed too. Plus, any attempt to m_shm_session->lend_arena()
* (by us below) or ->lend_object() or ->borrow_object() (when transmitting SHM-allocated objects process-to-process
* in the session) will fail (returning special failure values). And even those caveats (again) only matter
* after *setup_done; before *setup_done we will absolutely catch the problem before exiting the present method. */
Base::async_worker()->post([this, setup_done = std::move(setup_done), err_code]()
{
if (*setup_done)
{
FLOW_LOG_WARNING("Session [" << * this << "]: Internal-use (for SHM) channel reported outgoing-direction "
"error [" << err_code << "] [" << err_code.message() << "]. This occured after SHM-setup; "
"almost certainly the session master channel will catch a problem or has caught it; "
"almost certainly the session master channel and/or attempts to lend/borrow "
"will catch a problem or have caught it; "
"session will be hosed, or session opening will fail, depending on the situation.");
}
else
Expand Down Expand Up @@ -559,25 +562,15 @@ template<typename T>
typename CLASS_JEM_SESSION_IMPL::Blob
CLASS_JEM_SESSION_IMPL::lend_object(const typename Arena::template Handle<T>& handle)
{
auto blob = shm_session()->template lend_object<T>(handle);

// @todo Maybe we should just return empty Blob? For now doing this: SHM-classic will assert() too. Reconsider both.
assert((!blob.empty()) && "Most likely `handle` did not come validly from one of `*this`-owned `Arena`s.");

return blob;
return shm_session()->template lend_object<T>(handle);
}

TEMPLATE_JEM_SESSION_IMPL
template<typename T>
typename CLASS_JEM_SESSION_IMPL::Arena::template Handle<T>
CLASS_JEM_SESSION_IMPL::borrow_object(const Blob& serialization)
{
const auto obj = shm_session()->template borrow_object<T>(serialization);

// @todo Maybe we should just return nullptr? For now doing this: SHM-classic will assert() too. Reconsider both.
assert(obj && "Most likely `serialization` is invalid.");

return obj;
return shm_session()->template borrow_object<T>(serialization);
}

TEMPLATE_JEM_SESSION_IMPL
Expand Down
4 changes: 4 additions & 0 deletions src/ipc/session/shm/arena_lend/jemalloc/server_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ namespace ipc::session::shm::arena_lend::jemalloc
* arena-sharing SHM-provider by definition both sides allocate and thus write to the same SHM pool(s).
* However this comes at the expense of safety. Further discussion is up to ipc::shm docs.)
*
* @note Be ready for Session_mv::lend_object() to return an empty blob and Session_mv::borrow_object() to return
* a null pointer, both indicating an error: the session is hosed (opposing process likely down). See their
* doc headers for a bit more info on this.
*
* By using `shm::stl::Arena_activator<Arena>` (alias shm::arena_lend::jemalloc::Ipc_arena_activator) and
* #Allocator (alias shm::arena_lend::jemalloc::Ipc_arena_allocator)
* it is possible to construct and transmit not just POD (Plain Old Datatype) objects but combinations of those
Expand Down
16 changes: 14 additions & 2 deletions src/ipc/session/shm/arena_lend/jemalloc/session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,22 @@ class Session_mv :
* However, if your code specifically counts on `*this` being a shm::arena_lend::jemalloc::Server_session, then
* it is not wrong to rely on this knowledge.
*
* ### Possibility of error ###
* This method returns an `.empty()` blob in the event of error. Assuming you used it with proper inputs, this
* indicates the session is hosed -- most likely the opposing process is down -- and therefore your code should
* be ready for this absolutely possible eventuality. The session-hosed condition *will* shortly be indicated via
* session on-error handler as well. Informally we suggest handling the first sign of the session going down (whether
* this returning empty, borrow_object() returning null, or the session on-error handler firing) uniformly by
* abandoning the entire session ASAP.
*
* @tparam T
* See #Arena.
* @param handle
* Value returned by #Arena method `construct<T>()`. Note this is a mere `shared_ptr<T>` albeit with
* unspecified custom deleter logic attached. The #Arena instance can be session_shm(),
* Server_session_mv::app_shm(), or a non ipc::session-managed custom #Arena, as long as it has been
* registered via `shm_session()->lend_arena()`.
* @return See above. Never `.empty()`.
* @return See above. On success, non-empty; otherwise an `.empty()` blob.
*/
template<typename T>
Blob lend_object(const typename Arena::template Handle<T>& handle);
Expand All @@ -229,11 +237,15 @@ class Session_mv :
* Completes the cross-process operation begun by oppsing Session_mv::lend_object() that returned `serialization`;
* to be invoked in the intended new owner process which is operating `*this`.
*
* ### Possibility of error ###
* Indicated by this returning null, the remarks in the lend_object() doc header "Possibility of error" section
* apply here equally. Reminder: be ready for this to return null; it can absolutely happen.
*
* @tparam T
* See lend_object().
* @param serialization
* Value, not `.empty()`, returned by opposing lend_object() and transmitted bit-for-bit to this process.
* @return See above. Never null.
* @return See above. On success, non-null pointer; otherwise a null pointer.
*/
template<typename T>
typename Arena::template Handle<T> borrow_object(const Blob& serialization);
Expand Down
14 changes: 10 additions & 4 deletions src/ipc/session/standalone/shm/arena_lend/jemalloc/shm_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,14 @@ class Shm_session :
* #borrow_object. The serialization contents include information on how to deserialize the object and not
* contents of the object itself.
*
* @warning Be ready for this method returning empty blob. Assuming you provided proper inputs, this indicates session
* is hosed; in particular no further lending/borrowing is likely to work. Most likely the opposing process
* is down or chose to close the session.
*
* @param object The object to be lent.
*
* @return If successful, the serialized handle of the object, which should be transmitted by the caller (lender)
* and deserialized by the borrower. If unsuccessful, an empty blob. This would only fail if the object
* was previously registered.
* and deserialized by the borrower. If unsuccessful, an empty blob.
*/
template <typename T>
Blob lend_object(const std::shared_ptr<T>& object);
Expand All @@ -145,11 +148,14 @@ class Shm_session :
* The caller must have prior knowledge of what type the serialized object is. When the object is released (i.e.,
* shared pointer handle count drops to zero), the object will be deregistered and communicated back to the lender.
*
* @warning Be ready for this method returning null. Assuming you provided proper inputs, this indicates the session
* is hosed; in particular no further lending/borrowing is likely to work. Most likely the opposing process
* is down or chose to close the session.
*
* @tparam T The object type that is being borrowed.
* @param serialized_object A serialized handle of an object, which was previously serialized via #lend_object.
*
* @return If successful, the (deserialized) object. If unsuccessful, a null object. Failure would only occur if
* the size of the serialized object is insufficient or the (deserialized) object was previously registered.
* @return If successful, the (deserialized) object. If unsuccessful, a null object.
*/
template <typename T>
std::shared_ptr<T> borrow_object(const Blob& serialized_object);
Expand Down