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

OS_TaskExit() on VxWorks causes task table owner to be the deleted task #645

Closed
johnphamngc opened this issue Nov 5, 2020 · 1 comment · Fixed by #704 or #750
Closed

OS_TaskExit() on VxWorks causes task table owner to be the deleted task #645

johnphamngc opened this issue Nov 5, 2020 · 1 comment · Fixed by #704 or #750
Assignees
Labels
Milestone

Comments

@johnphamngc
Copy link

Describe the bug
OS_TaskExit() calls OS_ObjectIdFinalizeDelete(). This function sets the task's record->active_id to 0, which is also being used as the task ID on VxWorks. ObjectIdFinalizeDelete() subsequently calls OS_Unlock_Global, which errors out and does not clear the owner, as the task ID is now zeroed out and does not match the owner of the table. This does not occur on Linux

To Reproduce
Steps to reproduce the behavior:
On VxWorks, call CFE_ES_ExitChildTask with OSAL_CONFIG_DEBUG_PRINTF enabled

Expected behavior
Error message should not be printed out

Code snips

/*----------------------------------------------------------------
Function: OS_ObjectIdFinalizeDelete
Purpose: Helper routine, not part of OSAL public API.
See description in prototype
------------------------------------------------------------------*/
int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_common_record_t *record)
{
uint32 idtype = OS_ObjectIdToType_Impl(record->active_id);
osal_id_t callback_id;
/* Clear the OSAL ID if successful - this returns the record to the pool */
if (operation_status == OS_SUCCESS)
{
callback_id = record->active_id;
record->active_id = OS_OBJECT_ID_UNDEFINED;
}
else
{
callback_id = OS_OBJECT_ID_UNDEFINED;
}
/* Either way we must unlock the object type */
OS_Unlock_Global(idtype);
/* Give event callback to the application */
if (OS_ObjectIdDefined(callback_id))
{
OS_NotifyEvent(OS_EVENT_RESOURCE_DELETED, callback_id, NULL);
}
return operation_status;
}

void OS_Unlock_Global(uint32 idtype)
{
int32 return_code;
osal_id_t self_task_id;
OS_objtype_state_t *objtype;
if (idtype < OS_OBJECT_TYPE_USER)
{
objtype = &OS_objtype_state[idtype];
self_task_id = OS_TaskGetId_Impl();
/*
* Un-track ownership of this table. It should only be owned by one
* task at a time, and this aids in recovery if the owning task is
* deleted or experiences an exception causing it to not be freed.
*
* This is done before unlocking, while this has exclusive access
* to the state object.
*/
if (!OS_ObjectIdDefined(self_task_id))
{
/*
* This just means the calling context is not an OSAL-created task.
* This is not necessarily an error, but it should be tracked.
* Also note that the root/initial task also does not have an ID.
*/
self_task_id = OS_OBJECT_ID_RESERVED; /* nonzero, but also won't alias a known task */
}
if (!OS_ObjectIdEqual(objtype->table_owner, self_task_id))
{
/* this is almost certainly a bug */
OS_DEBUG("ERROR: global %u released by task 0x%lx when owned by task 0x%lx\n", (unsigned int)idtype,
OS_ObjectIdToInteger(self_task_id), OS_ObjectIdToInteger(objtype->table_owner));
}
else
{
objtype->table_owner = OS_OBJECT_ID_UNDEFINED;
}
return_code = OS_Unlock_Global_Impl(idtype);
}
else
{
return_code = OS_ERR_INCORRECT_OBJ_TYPE;
}
if (return_code != OS_SUCCESS)
{
OS_DEBUG("ERROR: unable to unlock global %u, error=%d\n", (unsigned int)idtype, (int)return_code);
}
}

/*----------------------------------------------------------------
*
* Function: OS_TaskGetId_Impl
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
osal_id_t OS_TaskGetId_Impl(void)
{
OS_impl_task_internal_record_t *lrec;
size_t index;
osal_id_t id;
id = OS_OBJECT_ID_UNDEFINED;
lrec = (OS_impl_task_internal_record_t *)taskTcb(taskIdSelf());
if (lrec != NULL)
{
index = lrec - &OS_impl_task_table[0];
if (index < OS_MAX_TASKS)
{
id = OS_global_task_table[index].active_id;
}
}
return id;
} /* end OS_TaskGetId_Impl */

/*----------------------------------------------------------------
*
* Function: OS_TaskGetId_Impl
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
osal_id_t OS_TaskGetId_Impl(void)
{
OS_U32ValueWrapper_t self_record;
self_record.opaque_arg = pthread_getspecific(POSIX_GlobalVars.ThreadKey);
return (self_record.id);
} /* end OS_TaskGetId_Impl */

System observed on:

  • SP0
  • OS: VxWorks 6.9
  • Versions: CFE 6.8, OSAL 5.1.0-rc1+dev16,

Additional context
See attached screenshots for stack traces
image
image
Reporter Info
John N. Pham, Northrop Grumman

@jphickey jphickey self-assigned this Nov 5, 2020
@jphickey
Copy link
Contributor

jphickey commented Nov 5, 2020

Although the tickets don't seem related, what I'm working on for #642 will likely implement a process where the deleted task record is only "marked" as deleted but keeps its ID valid while it waits for the task to actually exit.

So - it should fix this too, because the ID in the global record will still be valid when the unlocking is done.

@skliper skliper added this to the 6.0.0 milestone Nov 5, 2020
jphickey added a commit to jphickey/osal that referenced this issue Dec 22, 2020
Implement an "unlock key" - based on task ID - which can be part of the local
token, rather than relying on the task ID being the same between the lock
and unlock ops.  Notably, the task ID can change, in particular if the task
is exiting.

Also Fixes nasa#701, other general cleanup

Implement all global lock/unlock error checking in shared layer, not in
impl layer, for consistency.  Remove redundant checks.

Make all functions return void (should never fail) and in the
unlikely event that something does fail then report the error,
but no other recourse possible.
@skliper skliper added the bug label Jan 4, 2021
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants