dcmtk
dcmtk copied to clipboard
Second sketch for mechanism to cancel DcmSCU negotiateAssociation before connectionTimeout elapses
If a server is unavailable, the DcmSCU::negotiateAssociation function call will block for the duration of the connectionTimeout. In many cases this timeout may be long, 30-60 seconds to ensure that we can robustly establish associations against slow servers. This is a long time for a user to wait for a device to shut down.
In some use cases, for example for portable imaging devices, the ability to quickly shut down is important to reduce the risk of the user pulling the power cable before the system is completely shut down. Such devices typically have a high risk that servers are temporarily unavailable, for example if they are serving multiple purposes and have to be moved from lab to lab.
This pull request adds support for early cancellation of TCP connect attempts by introducing a TCP poll interval and a cancellation callback to DUL_ASSOCIATESERVICEPARAMETERS. The requestAssociationTCP function will call the cancellation callback at intervals specified by the polling interval and allows the caller to decide if the TCP connect attempt should be cancelled.
On the DcmSCU level, DcmSCU::negotiateAssociation is changed to take a cancelation token as an optional argument, and allows the client to implement its own cancellation mechanism.
This draft has some weaknesses:
- The effective timeout can be longer than the connection timeout because the lowest granularity is the poll interval (should be possible to fix by modifying the poll interval in the last iteration.
- There is no sanity check of the poll interval. It should be shorter than the ConnectTimeout
- The new variables in DUL_ASSOCIATESERVICEPARAMETERS are not really related to associations, but rather to network. Should they be moved elsewhere?
- Most programmers may not care about TCP cancellation, which is only needed in very particular use cases. Changing DcmSCU::negotiateAssociation by adding a cancellation token complicates the API for everyone. The advantage is that the lifetime requirements for the cancelation token becomes very clear (it has to outlive the call to negotiateAssociation). A commercial alternative to DCMTK recently introduced a similar callback-based approach to enable TCP cancellation.
- The change adds complexity to requestAssociationTCP which is already very complex. Can we do such a change safely without refactoring the dulfsm.cc into a better shape?
- The tests attempts to find an available unused port using an SCP, but does not take ownership of it. This may cause intermittently failing unit tests on a heavily loaded build node running many tests in parallel.