Update key scheme for the active timers map

[sonndinh]

No description provided.

[mitza-oci]

This looks good, but I do have some questions:

• Is a single ACE_Reactor object used for these timers, or is the problem in considering Timer IDs that originate from different reactors?
• Is a single event handler object receiving the handle_timeout for different logical timers and then de-multiplexing on its own? If so, the arg argument to schedule_timer should be something meaningful in the application space (as it seems like you've done here with the latest changes) and not a Timer ID assigned by the reactor.

[sonndinh]

This looks good, but I do have some questions:

• Is a single ACE_Reactor object used for these timers, or is the problem in considering Timer IDs that originate from different reactors?

Each TimerHandler instance holds an ACE_Reactor object. So in this case, the ControllerSelector object has a single reactor object. A power device can have another reactor object for handling the Handshaking functions. But this PR addresses an issue with the timers on the reactor of ControllerSelector.

• Is a single event handler object receiving the handle_timeout for different logical timers and then de-multiplexing on its own? If so, the arg argument to schedule_timer should be something meaningful in the application space (as it seems like you've done here with the latest changes) and not a Timer ID assigned by the reactor.

Yes, there is a single handle_timeout in the TimerHandler class, and it dispatches to the corresponding callback depending on the timer kind. The existing method uses timer Ids returned by ACE_Reactor as the keys into a map for looking up the correct timer. However, the issue was that the timer Ids are not unique, and so using them to index the map causes a problem. This comment gives a more specific explanation: https://github.com/OpenDDS/DeveloperResources/pull/24/files#diff-ea293d0d8ffb2e6b8ed3b29993606bd9809de5c0e1b133f5bc46e3814f71e55bR39-R47
This fixes this by using an application-specific key scheme, passed through the handle_timeout method, like you suggested.

[mitza-oci]

OK. What I found from reviewing the code is that ACE's Timer IDs are actually unique, except that when they're not needed anymore internally they are recycled. So the scope of uniqueness is only from when the schedule_timer happens until the moment it expires (if not repeating on an interval) or is canceled.

[sonndinh]

OK. What I found from reviewing the code is that ACE's Timer IDs are actually unique, except that when they're not needed anymore internally they are recycled. So the scope of uniqueness is only from when the schedule_timer happens until the moment it expires (if not repeating on an interval) or is canceled.

That's right. The uniqueness I mentioned throughout the PR is from the application perspective/requirements. The timer Ids are indeed unique in the direct context/use of ACE.