Merge pull request #53 from Flow-IPC/sess_tkt29_shm_sess_lend_brw_fai…

…l_plus_polish

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.

src/ipc/session/detail/shm/arena_lend/jemalloc/server_session_impl.hpp

@@ -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.

src/ipc/session/detail/shm/arena_lend/jemalloc/session_impl.hpp

@@ -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
@@ -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

src/ipc/session/shm/arena_lend/jemalloc/server_session.hpp

@@ -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

src/ipc/session/shm/arena_lend/jemalloc/session.hpp

@@ -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);
@@ -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);

src/ipc/session/standalone/shm/arena_lend/jemalloc/shm_session.hpp

@@ -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);
@@ -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);