multiple calls to cm_register_transition() for the same transition

[dd1]

cm_register_transition() allows registering the same transition (i.e. TR_START) multiple times (maybe with different handler functions). This can be used to implement “pre-start” (sequence number 1, called before anybody else) and “post-start” (sequence number 999, called after everybody else).

However, when the same sequence number is used to register the same transition multiple times (i.e. TR_STARTABORT handler is registered multiple times, by mistake), the multithreaded transition code becomes confused (complicated issues caused by two different threads trying to call the same client at the same time, ask me about it).

I think cm_register_transition() should issue a warning or error if the same transition is registered twice with the same sequence number.

Additional things:

• cm_deregister_transition() will remove all of the multiple requests from ODB (good), but not from “_trans___table” (probably bad)
• cm_set_transition_sequence() will drop all but the 1st of multiple-registered transitions in ODB by resetting the array length to 1. (bad). not sure if it does the correct thing in “_trans_table”.
• cm_transition_call_direct() will always call the 1st handler for multiple-registered transitions (handler from the first entry in “_trans_table” with matching transition type). (bad)
• cm_transition_dispatch() is correct (good), but for multiple-registered transitions that all use the same sequence number (i.e. TR_STARTABORT registered twice with sequence number 500), the first handler will be called always. (bad). this cannot be fixed.
• transition.html does not know about multiple-registered transitions and will display them incorrectly (bad)

Because cm_transition_dispatch() cannot be fixed (it cannot know if TR_STARTABORT with sequence number 500 is called for the 1st or the 2nd time), I think we should not permit this situation. cm_register_transition() should refuse registering the same transition with the same sequence number twice.

cm_transition_call_direct() is used by single-threaded transition (because single-threaded programs like mlogger cannot call their own tr_start() and tr_stop() through the RPC). right now it will not work correctly, but it is fixable and should be fixed.

cm_set_transition_sequence(TR_START) is not well defined if TR_START is registered multiple times for “pre-start” and “post-start”. I think it should issue an error message and do nothing in this case (instead of dropping all registrations except for the 1st one from ODB).

transition.html should be fixed, this is easy.

K.O.

[dd1]

partial fixes on branch feature/buffer_mutex - cm_transition() ignores duplicate registration (same transition, same sequence number), transition.html displays multiple-registered transitions correctly (same client, same transition, different sequence number). a4e1363567ecc7d51f1dc4f7f73d47555075ab9b and dc1422ca12dae77fa002d7642f42ab239d53c99b. K.O.

[dd1]

status update. changes merged to develop:

• cm_transition() will warn about duplicates (same client, same transition, same sequence number) and do the RPC call only once.
• cm_register_transition() should complain about duplicates, but does not.
• “pre-start”, “post-start” transitions should work, transition page will display them correctly.
• other buglets reported above, status unchanged.

K.O.

‌

[dd1]

cm_transition_call_direct() is now correct. commit 7f1cff226e56220892157526fabfb7a32f58f4db. K.O.

[dd1]

cm_register_transition(), cm_deregister_transition() and cm_set_transition_sequence() are now correct. commit e6f41b36f772535e50cc71a85e378406f5ed79a1. K.O.

[dd1]

access to tr_fifo is also thread safe now.

[dd1]

all good now.