Skip to content

Commit

Permalink
Implement delayed enabling of entities using dds_enable()
Browse files Browse the repository at this point in the history
Signed-off-by: TheFixer <thefixer@iteazz.com>

This pull request implements delayed enabling of entities, in particular
of publishers, subscribers, readers and writers. Delayed enabling allows
immutable qos changes to be changed as long as the entity is not enabled,
but once the entity is enabled immutable qos changes are not allowed anymore.

Delayed enabling is implemented by deferring the creation of ddsi entities as
long as the entity is not enabled. The application can call dds_enable() on a
disabled entity to enable it. Calling dds_enable() on an already enabled entity
is considered a noop for the application.

This pull request implements the following behaviour:

- Participants are always enabled.
- Created entities will be automatically enabled when their parent entity is enabled,
  and their parent has autoenable set to true. This is the default.
- Calling dds_create_writer() on a disabled publisher which has autoenable=true will
  return a dds writer without a corresponding ddsi writer (and similar for readers)
- Disabled entities can be enabled by calling dds_enable() on the entity.
  When the parent of the entity is not enabled, enabling will fail and return with
  DDS_RETCODE_PRECONDITION_NOT_MET. When the entity cannot be enabled because of qos
  mismatches or invalid security credentials, an error code will be returned.
  The call dds_enable() will return DDS_RETCODE_OK if is has enabled the entity
  and alls its disabled children that have been created while their parent has
  autoenable=true. If not all children are successfully enabled then dds_enable()
  will return with the error code of the first child that that could not be enabled.
- Calling dds_enable() on an already enabled entity is a noop.
- The m_iid is pre-allocated when a dds_entity is created, and is reused when the
  corresponding ddsi entity gets enabled.
- Immutable QoS settings can be changed on an entity as long as the entity is not enabled.
  As a consequence the creation of some qos dependent structures (like whc) have been
  moved from the dds entity to the ddsi entity.
- The whc is created when entity is enabled, not prior to that. The same holds for the
  xpack creation.
- When readers that have no predefined rhc, the rhc will be created when the reader gets
  enabled (i.e., the call to dds_rhc_default_new() is done when the entity is enabled)
- Readers that do have a predefined rhc will use this rhc when becoming enabled.
- thread_state_awake() and thread_state_sleep() functions have moved to the location where
  entities are enabled.
- The check for the availability of the security credentials has been moved to location
  where entities are enabled. This implies that a disabled dds writer or reader can be
  created before checking its security credentials. To not change legacy behaviour the
  creation of a dds reader or writer fails with DDS_RETCODE_NOT_ALLOWED_BY_SECURITY
  when the reader or writer is automatically enabled when created.
  Note that this can only happen for writers whose parent is enabled and have autoenabled
  set to true.
- If an entity (e.g., subscriber) is enabled which causes recursive calls to dds_enable()
  on multiple children (e.g., readers), and some of these child entities are successfully
  enabled and others are not (e.g., because the some of the readers have the correct security
  credentials and others have not) then the dds_enable() on the parent will fail. It is
  the responsibility of the application to find out which of the entities have been
  enabled, and which have not.
- When a reader is created with an implicit subscriber, then the implicit subscriber is
  enabled if and only if the autoenable setting of the participant is set to true.
  Similarly for the creation of a writer with an implicit publisher.
- Various tests have been implemented.

Notes for the reviewer.
- If a participant has autoenable=false and an reader (or writer) with an implicit
  subscriber (or publisher) is created, I don't think it is possible to enable the
  implicit subscriber (or publisher)( anymore. I therefore think it would be wise
  to prevent the creation of a reader (or writer) with an implicit subscriber
  (or publisher) when the participant has autoenable=false, and return
  DDS_RETCODE_PRECONDITION_NOT_MET in that case. Do you agree?
- Currently, I don't use locking when recursively enabling children. Please check.
- I am not sure, but I have the impression that some security tests take much time.
  Not sure if it is caused by this pull request.
  • Loading branch information
TheFixer authored and eboasson committed Feb 8, 2021
1 parent 81666fd commit 7d55316
Show file tree
Hide file tree
Showing 26 changed files with 1,808 additions and 216 deletions.
86 changes: 57 additions & 29 deletions src/core/ddsc/include/dds/dds.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,46 +224,50 @@ dds_builtintopic_endpoint_t;
*/

/**
* @brief Enable entity.
* @brief Enable an entity.
*
* @note Delayed entity enabling is not supported yet (CHAM-96).
* Entities can be created in an enabled or disabled state.
* Enabled entities are immediately activated at creation time, and
* their immutable QoS settings cannot be changed.
* Disabled entities are not yet activated at creation time,
* so it is still possible to change their immutable QoS settings.
*
* This operation enables the dds_entity_t. Created dds_entity_t objects can start in
* either an enabled or disabled state. This is controlled by the value of the
* entityfactory policy on the corresponding parent entity for the given
* entity. Enabled entities are immediately activated at creation time meaning
* all their immutable QoS settings can no longer be changed. Disabled Entities are not
* yet activated, so it is still possible to change their immutable QoS settings. However,
* once activated the immutable QoS settings can no longer be changed.
* Creating disabled entities can make sense when the creator of the DDS_Entity
* does not yet know which QoS settings to apply, thus allowing another piece of code
* to set the QoS later on.
*
* The default setting of DDS_EntityFactoryQosPolicy is such that, by default,
* entities are created in an enabled state so that it is not necessary to explicitly call
* dds_enable on newly-created entities.
*
* The dds_enable operation produces the same results no matter how
* many times it is performed. Calling dds_enable on an already
* enabled DDS_Entity returns DDS_RETCODE_OK and has no effect.
* Typically, subscribers, publishers, readers and writers can be created in a disabled state.
* This is controlled by the value of the DDS_EntityFactoryQosPolicy of the parent
* of the entity. The default setting of DDS_EntityFactoryQosPolicy is such that
* entities are created in an enabled state.
*
* If an Entity has not yet been enabled, the only operations that can be invoked
* on it are: the ones to set, get or copy the QosPolicy settings, the ones that set
* (or get) the Listener, the ones that get the Status and the dds_get_status_changes
* operation (although the status of a disabled entity never changes). Other operations
* will return the error DDS_RETCODE_NOT_ENABLED.
*
* Entities created with a parent that is disabled, are created disabled regardless of
* the setting of the entityfactory policy.
*
* If the entityfactory policy has autoenable_created_entities
* set to TRUE, the dds_enable operation on the parent will
* automatically enable all child entities created with the parent.
*
* The Listeners associated with an Entity are not called until the
* Entity is enabled. Conditions associated with an Entity that
* is not enabled are "inactive", that is, have a trigger_value which is FALSE.
*
* Entities created with a parent that is disabled, are created disabled regardless of
* the setting of the DDS_EntityFactoryQosPolicy.
*
* Calling dds_enable() on a disabled DDS_Entity enables the DDS_Entity only if its
* parent entity is enabled. When the DDS Entity is enabled, the dds_enable() operation
* will also automatically enable all child entities of the DDS_Entity that have been
* created when the DDS_EntityFactoryQosPolicy of the DDS_Entity has
* autoenable_created_entities set to TRUE. For example, if a disabled subscriber
* which has its autoenable_created_entities set to TRUE is used to create 3 readers,
* then these 3 readers will be enabled automatically as soon as the subscriber is
* enabled.
*
* The dds_enable() operation will return DDS_RETCODE_OK if an entity and all its
* autoenabled children have been enabled successfully. In case one or more of
* its autoenabled children could not be enabled, an error code is returned.
* It is the responsibility of the application to find out which of the children
* code were enabled, and which not enabled.
*
* Calling dds_enable() on an already enabled DDS_Entity returns DDS_RETCODE_OK and
* has no effect.
*
* @param[in] entity The entity to enable.
*
* @returns A dds_return_t indicating success or failure.
Expand All @@ -279,6 +283,8 @@ dds_builtintopic_endpoint_t;
* The entity has already been deleted.
* @retval DDS_RETCODE_PRECONDITION_NOT_MET
* The parent of the given Entity is not enabled.
* @retval DDS_RETCODE_NOT_ALLOWED_BY_SECURITY
* The security plugin prevents enabling of the Entity.
*/
DDS_EXPORT dds_return_t
dds_enable(dds_entity_t entity);
Expand Down Expand Up @@ -1521,6 +1527,16 @@ dds_wait_for_acks(dds_entity_t publisher_or_writer, dds_duration_t timeout);
* A valid reader handle.
* @retval DDS_RETCODE_ERROR
* An internal error occurred.
* @retval DDS_RETCODE_ILLEGAL_OPERATION
* The provided entity is not a participant or subscriber.
* @retval DDS_RETCODE_BAD_PARAMETER
* One of the given arguments is not valid.
* @retval DDS_RETCODE_INCONSISTENT_POLICY
* The provided qos is not allowed.
* @retval DDS_RETCODE_NOT_ALLOWED_BY_SECURITY
* The security plugin prevents creation of the reader.
* @retval DDS_RETCODE_OUT_OF_RESOURCES
* There are not enough resources available to create the reader.
*/
/* TODO: Complete list of error codes */
DDS_EXPORT dds_entity_t
Expand Down Expand Up @@ -1593,9 +1609,21 @@ dds_reader_wait_for_historical_data(
* @returns A valid writer handle or an error code.
*
* @returns >0
* A valid writer handle.
* A valid writer handle.
* @returns DDS_RETCODE_ERROR
* An internal error occurred.
* An internal error occurred.
* @retval DDS_RETCODE_ERROR
* An internal error occurred.
* @retval DDS_RETCODE_ILLEGAL_OPERATION
* The provided entity is not a participant or subscriber.
* @retval DDS_RETCODE_BAD_PARAMETER
* One of the given arguments is not valid.
* @retval DDS_RETCODE_INCONSISTENT_POLICY
* The provided qos is not allowed.
* @retval DDS_RETCODE_NOT_ALLOWED_BY_SECURITY
* The security plugin prevents creation of the writer.
* @retval DDS_RETCODE_OUT_OF_RESOURCES
* There are not enough resources available to create the writer.
*/
/* TODO: Complete list of error codes */
DDS_EXPORT dds_entity_t
Expand Down
23 changes: 23 additions & 0 deletions src/core/ddsc/include/dds/ddsc/dds_public_qos.h
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,17 @@ dds_qunset_bprop (
dds_qos_t * __restrict qos,
const char * name);

/* @brief Set the entity-factory policy of a qos structure
*
* @param[in,out] qos - Pointer to a dds_qos_t structure that will store the policy
* @param[in] autoenable - True iff entities that are created from the factory for which
* this qos is applied are enabled
*/
DDS_EXPORT void
dds_qset_entity_factory (
dds_qos_t * __restrict qos,
bool autoenable);

/**
* @brief Set the type consistency enforcement policy of a qos structure
*
Expand Down Expand Up @@ -866,6 +877,18 @@ dds_qget_type_consistency (
bool *prevent_type_widening,
bool *force_type_validation);

/* @brief Get the entity-factory qos policy
*
* @param[in] qos - Pointer to a dds_qos_t structure storing the policy
* @param[in,out] autoenable - Pointer that will store whether to enable child entities when created
*
* @returns - false iff any of the arguments is invalid or the requested qos value is not
* present in the qos object
*/
DDS_EXPORT bool dds_qget_entity_factory (
const dds_qos_t * __restrict qos,
bool *autoenable);

#if defined (__cplusplus)
}
#endif
Expand Down
7 changes: 6 additions & 1 deletion src/core/ddsc/src/dds__entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
extern "C" {
#endif

DDS_EXPORT dds_return_t dds_entity_autoenable_children (dds_entity *entity);

DDS_EXPORT dds_entity_t
dds_entity_init(
dds_entity * e,
Expand Down Expand Up @@ -68,7 +70,7 @@ DDS_EXPORT inline dds_entity *dds_entity_from_handle_link (struct dds_handle_lin
}

DDS_EXPORT inline bool dds_entity_is_enabled (const dds_entity *e) {
return (e->m_flags & DDS_ENTITY_ENABLED) != 0;
return ((e->m_flags & DDS_ENTITY_ENABLED) != 0);
}

DDS_EXPORT void dds_entity_status_set (dds_entity *e, status_mask_t t);
Expand Down Expand Up @@ -176,6 +178,9 @@ dds_generic_unimplemented_operation(
dds_entity_t handle,
dds_entity_kind_t kind);

DDS_EXPORT bool
dds_entity_creation_allowed (
const dds_return_t rc);

#if defined (__cplusplus)
}
Expand Down
11 changes: 9 additions & 2 deletions src/core/ddsc/src/dds__types.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ struct dds_listener {

/* Entity flag values */

#define DDS_ENTITY_ENABLED ((uint32_t) 0x1) /* DDS "enabled" state */
#define DDS_ENTITY_IMPLICIT ((uint32_t) 0x2) /* implicit ones get deleted when the last child is deleted */
#define DDS_ENTITY_ENABLED ((uint32_t) 0x1) /* DDS "enabled" state */
#define DDS_ENTITY_IMPLICIT ((uint32_t) 0x2) /* implicit ones get deleted when the last child is deleted */
#define DDS_ENTITY_ENABLE_ON_PARENT ((uint32_t) 0x4) /* flag that indicates that the entity was created by a parent
with autoenable=true while the parent was disabled */

struct dds_domain;
struct dds_entity;
Expand All @@ -108,6 +110,7 @@ typedef struct dds_entity_deriver {
dds_return_t (*validate_status) (uint32_t mask);
struct dds_statistics * (*create_statistics) (const struct dds_entity *e);
void (*refresh_statistics) (const struct dds_entity *e, struct dds_statistics *s);
dds_return_t (*enable) (struct dds_entity *e) ddsrt_nonnull_all;
} dds_entity_deriver;

struct dds_waitset;
Expand Down Expand Up @@ -182,6 +185,7 @@ dds_return_t dds_entity_deriver_dummy_set_qos (struct dds_entity *e, const dds_q
dds_return_t dds_entity_deriver_dummy_validate_status (uint32_t mask);
struct dds_statistics *dds_entity_deriver_dummy_create_statistics (const struct dds_entity *e);
void dds_entity_deriver_dummy_refresh_statistics (const struct dds_entity *e, struct dds_statistics *s);
dds_return_t dds_entity_deriver_dummy_enable (struct dds_entity *e);

inline void dds_entity_deriver_interrupt (struct dds_entity *e) {
(dds_entity_deriver_table[e->m_kind]->interrupt) (e);
Expand Down Expand Up @@ -210,6 +214,9 @@ inline struct dds_statistics *dds_entity_deriver_create_statistics (const struct
inline void dds_entity_deriver_refresh_statistics (const struct dds_entity *e, struct dds_statistics *s) {
dds_entity_deriver_table[e->m_kind]->refresh_statistics (e, s);
}
inline dds_return_t dds_entity_deriver_enable (struct dds_entity *e) {
return dds_entity_deriver_table[e->m_kind]->enable (e);
}

typedef struct dds_cyclonedds_entity {
struct dds_entity m_entity;
Expand Down
18 changes: 18 additions & 0 deletions src/core/ddsc/src/dds_builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,23 @@ static struct ddsi_tkmap_instance *dds__builtin_get_tkmap_entry (const struct dd
return tk;
}

static struct ddsi_tkmap_instance *dds__builtin_get_tkmap_entry_with_iid (const struct ddsi_guid *guid, const uint64_t iid, void *vdomain)
{
struct dds_domain *domain = vdomain;
struct ddsi_tkmap_instance *tk;
struct ddsi_serdata *sd;
union { ddsi_guid_t guid; struct ddsi_keyhash keyhash; } x;
x.guid = nn_hton_guid (*guid);
/* any random builtin topic will do (provided it has a GUID for a key), because what matters is the "class"
of the topic, not the actual topic; also, this is called early in the initialisation of the entity with
this GUID, which simply causes serdata_from_keyhash to create a key-only serdata because the key lookup
fails. */
sd = ddsi_serdata_from_keyhash (domain->builtin_participant_type, &x.keyhash);
tk = ddsi_tkmap_find_with_iid (domain->gv.m_tkmap, sd, iid, true);
ddsi_serdata_unref (sd);
return tk;
}

struct ddsi_serdata *dds__builtin_make_sample (const struct entity_common *e, ddsrt_wctime_t timestamp, bool alive)
{
/* initialize to avoid gcc warning ultimately caused by C's horrible type system */
Expand Down Expand Up @@ -273,6 +290,7 @@ void dds__builtin_init (struct dds_domain *dom)

dom->btif.arg = dom;
dom->btif.builtintopic_get_tkmap_entry = dds__builtin_get_tkmap_entry;
dom->btif.builtintopic_get_tkmap_entry_with_iid = dds__builtin_get_tkmap_entry_with_iid;
dom->btif.builtintopic_is_builtintopic = dds__builtin_is_builtintopic;
dom->btif.builtintopic_is_visible = dds__builtin_is_visible;
dom->btif.builtintopic_write = dds__builtin_write;
Expand Down
77 changes: 66 additions & 11 deletions src/core/ddsc/src/dds_entity.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ struct dds_statistics *dds_entity_deriver_dummy_create_statistics (const struct
void dds_entity_deriver_dummy_refresh_statistics (const struct dds_entity *e, struct dds_statistics *s) {
(void) e; (void) s;
}
dds_return_t dds_entity_deriver_dummy_enable (struct dds_entity *e) {
(void) e; return DDS_RETCODE_ILLEGAL_OPERATION;
}

extern inline void dds_entity_deriver_interrupt (struct dds_entity *e);
extern inline void dds_entity_deriver_close (struct dds_entity *e);
Expand All @@ -78,6 +81,7 @@ extern inline bool dds_entity_supports_set_qos (struct dds_entity *e);
extern inline bool dds_entity_supports_validate_status (struct dds_entity *e);
extern inline struct dds_statistics *dds_entity_deriver_create_statistics (const struct dds_entity *e);
extern inline void dds_entity_deriver_refresh_statistics (const struct dds_entity *e, struct dds_statistics *s);
extern inline dds_return_t dds_entity_deriver_enable (struct dds_entity *e);

static int compare_instance_handle (const void *va, const void *vb)
{
Expand Down Expand Up @@ -196,6 +200,46 @@ static bool entity_kind_has_qos (dds_entity_kind_t kind)
}
#endif

dds_return_t dds_entity_autoenable_children (dds_entity *e)
{
struct dds_entity *c;
ddsrt_avl_iter_t it;
dds_return_t rc = DDS_RETCODE_OK, ret;

assert(e);
assert(e->m_flags & DDS_ENTITY_ENABLED);

/* enable all disabled children with the flag DDS_ENTITY_ENABLE_ON_PARENT */
for (c = ddsrt_avl_iter_first (&dds_entity_children_td, &e->m_children, &it); c != NULL; c = ddsrt_avl_iter_next (&it)) {
if ((!(c->m_flags & DDS_ENTITY_ENABLED)) && (c->m_flags & DDS_ENTITY_ENABLE_ON_PARENT)) {
ret = dds_enable((dds_entity_t)c->m_hdllink.hdl);
if ((rc == DDS_RETCODE_OK) && (ret < 0)) {
rc = ret;
}
}
}
return rc;
}

static void dds_entity_init_mflags (dds_entity *e, dds_entity *parent, bool implicit)
{
assert (e);

if (implicit)
e->m_flags |= DDS_ENTITY_IMPLICIT;
/* entity is enabled by default unless parent is not enabled or parent has
* autoenable_created_entities set to false */
e->m_flags |= DDS_ENTITY_ENABLED;
if (parent) {
if (parent->m_qos && parent->m_qos->entity_factory.autoenable_created_entities)
/* parent has autoenable set to true */
e->m_flags |= DDS_ENTITY_ENABLE_ON_PARENT;
/* do not enable if parent is not enabled or no autoenable */
if ((!(parent->m_flags & DDS_ENTITY_ENABLED)) || ((e->m_flags & DDS_ENTITY_ENABLE_ON_PARENT) == 0))
e->m_flags &= ~DDS_ENTITY_ENABLED;
}
}

dds_entity_t dds_entity_init (dds_entity *e, dds_entity *parent, dds_entity_kind_t kind, bool implicit, dds_qos_t *qos, const dds_listener_t *listener, status_mask_t mask)
{
dds_handle_t handle;
Expand All @@ -211,10 +255,10 @@ dds_entity_t dds_entity_init (dds_entity *e, dds_entity *parent, dds_entity_kind
e->m_cb_pending_count = 0;
e->m_observers = NULL;

/* TODO: CHAM-96: Implement dynamic enabling of entity. */
e->m_flags |= DDS_ENTITY_ENABLED;
if (implicit)
e->m_flags |= DDS_ENTITY_IMPLICIT;
/* All entities are enabled by default, except if the parent is not enabled or
* if the parent's entity factory qos indicates that children of this parent
* should not be enabled when created */
dds_entity_init_mflags (e, parent, implicit);

/* set the status enable based on kind */
if (entity_has_status (e))
Expand Down Expand Up @@ -1059,15 +1103,11 @@ dds_return_t dds_enable (dds_entity_t entity)

if ((rc = dds_entity_lock (entity, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
return rc;

/* only enable entities that are not yet enabled */
if ((e->m_flags & DDS_ENTITY_ENABLED) == 0)
{
/* TODO: Really enable. */
e->m_flags |= DDS_ENTITY_ENABLED;
DDS_CERROR (&e->m_domain->gv.logconfig, "Delayed entity enabling is not supported\n");
}
rc = dds_entity_deriver_enable(e);
dds_entity_unlock (e);
return DDS_RETCODE_OK;
return rc;
}

dds_return_t dds_get_status_changes (dds_entity_t entity, uint32_t *status)
Expand Down Expand Up @@ -1327,10 +1367,15 @@ dds_return_t dds_triggered (dds_entity_t entity)

if ((ret = dds_entity_lock (entity, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
return ret;
if (!dds_entity_is_enabled(e)) {
ret = DDS_RETCODE_NOT_ENABLED;
goto err_enabled;
}
if (entity_has_status (e))
ret = ((ddsrt_atomic_ld32 (&e->m_status.m_status_and_mask) & SAM_STATUS_MASK) != 0);
else
ret = (ddsrt_atomic_ld32 (&e->m_status.m_trigger) != 0);
err_enabled:
dds_entity_unlock (e);
return ret;
}
Expand Down Expand Up @@ -1546,3 +1591,13 @@ dds_return_t dds_assert_liveliness (dds_entity_t entity)
dds_entity_unpin (e);
return rc;
}

bool dds_entity_creation_allowed (const dds_return_t rc)
{
switch (rc) {
case DDS_RETCODE_NOT_ALLOWED_BY_SECURITY:
return false;
default:
return true;
}
}
Loading

0 comments on commit 7d55316

Please sign in to comment.