On Wed May 15 23:08:18 2024 +0000, Jinoh Kang wrote:
While unlikely, I think this is a race. After checking id validity,
the object may be invalidated and replaced with something else with matching seq. If an object is invalidated and/or replaced with something else, the seq never matches because neither action resets the seq in the first place. This is the definition of a shared object:
typedef volatile struct { LONG64 seq; /* sequence number - server updating if (seq & 1) != 0 */ object_id_t id; /* object unique id, object data is valid if != 0 */ object_shm_t shm; /* object shared data */ } shared_object_t;Here, the `id` and `seq` are reset by `alloc_shared_object` and `free_shared_object`, but `seq` is left alone. For example, here is `free_shared_object`'s body:
SHARED_WRITE_BEGIN( &object->obj.shm, object_shm_t ) { mark_block_noaccess( (void *)shared, sizeof(*shared) ); CONTAINING_RECORD( shared, shared_object_t, shm )->id = 0; } SHARED_WRITE_END;The `seq` is incremented by `SHARED_WRITE_*` macros so the client would notice that something changed and re-tries reading.
Checking id after seq would not have that problem.
The object may have been invalidated between the unlock (checking last seq) and checking id. While this doesn't cause an issue in practice if every ID is assigned to an object once and only once, this is something to be explicitly aware about. Also, we'd need atomic access for id itself. Putting the id access in the seqlock (which is preserved across free/alloc of object) avoids this problem.
I see, if seq outlives id validity then that's indeed fine, thanks.