Incorrect call to OS_ObjectIdCompose_Impl in OS_TimerDelete

[jphickey]

Describe the bug
OSAL has an incorrect setting of first_cb when deleting timers.

osal/src/os/shared/src/osapi-time.c

Lines 432 to 436 in 1a82657

	if (local->next_ref != local_id)
	{
	OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TIMEBASE, local->next_ref,
	&OS_timebase_table[local->timebase_ref].first_cb);
	}

The second argument to OS_ObjectIdCompose_Impl() is a serial number, not a table index. These are only the same value until table entries start to be re-used, after this they become different, and this will start to fail. This should really not be using OS_ObjectIdCompose_Impl() at all here.

To Reproduce

• Create and delete several timebases - at least OS_MAX_TIMEBASES - such that table entries start to be re-used.
• Create another valid timebase for the test (do not delete).
• Create at least two timers based on this timebase
• Delete one of the timers.

At this point the ID in the timebase callback ring (first_cb member) may refer to an invalid entry - a timebase ID which does not exist.

Expected behavior
Should look up the active_id from the actual table entry instead - do not re-compose the ID, because next_ref is a table index, not a serial number.

System observed on:
Ubuntu 20.04

Additional context
It is only possible to trigger this after a rather extensive sequence of creating and deleting these resources. So this is probably unlikely to ever occur in a real system where timers are typically created and run forever. Should still be fixed though.

This was initially discovered by enforcing type-safety in the osal_index_t and osal_id_t - during this scrub it revealed that this was passing an osal_index_t to a function which is supposed to accept a serial number. So type safety = good.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

[jphickey]

Note - found an easier way to reproduce this error, which is to simply delete timers in a different order than they were created. Turns out all of the current functional tests create and delete in the same order, so it wasn't noticed.

Will update at least one test case to delete timers in a different order than they were created.

The recommended solution is to use the full object ID - not just the table index - to form the linked list of timer callbacks. This will be more robust and be simpler too, at least after #648 is done.

The reason #648 is relevant/related here is that it can provide a simpler and more reliable way to tracking table lock status via lock tokens. Without this it becomes difficult to track the lock status when using functions like OS_ObjectIdGetById inside other functions.