Add ability to abort association after receiving release request instead of sending release response

[dimrozakis]

Is your feature request related to a problem? Please describe.

I've implemented a store SCP using pynetdicom. After several C-STORE requests, the SCU sends an A-RELEASE request. It's at this point, after all DICOMs have been received, that the SCP needs to push all received DICOMs to a processing system which may or may not accept them. If the processing system rejects the data, I'd like the SCP to be able to notify the SCU that something didn't go as planned.

The idea I came up with, and please do feel free to suggest better alternatives, was to send a success response for each C-STORE and when the A-RELEASE request is received, the SCP would forward the data and if the processing system rejected them, the SCP would abort the association instead of confirming the release (see 7.2.2.6).

However, the EVT_RELEASED event is a notification event and is only triggered after the release response has been sent.

Describe the solution you'd like

Provide a way to abort an association after a release request has been received instead of responding to it by turning evt.EVT_RELEASED into an intervention event that can abort the association or adding a new event for this purpose.

Describe alternatives you've considered

I don't know of any other viable alternatives. The transfer has to happen with C-STOREs (for compatibility purposes), the actual outcome is only determined in the end, the SCU should be notified somehow. The SCU sends a release request right after finishing sending the C-STOREs. Aborting after it's received seems like the only opportunity to signal that something went wrong.

[scaramallion]

You can bind a handler to EVT_PDU_RECV, check to see if isinstance(event.pdu, pynetdicom.pdu.A_RELEASE_RQ) and manually abort.

[dimrozakis]

Thanks @scaramallion. I did it as you suggested (just had to use isinstance instead of equality check) and it works perfectly, thanks again.

[scaramallion]

Whoops, yeah isinstance... 😄

[scaramallion]

Another option is to call assoc.acse.is_release_requested() -> bool.

[pchristos]

Hi,

I am colleague of @dimrozakis and I have been looking into a memory leak, which seems to be related to the suggested approach of aborting the association.

If an error is thrown during the handling of the A_RELEASE_RQ and the association is aborted, the DUL thread is not killed, leading to a memory leak.

Minimal example:

def _on_pdu_recv(self, event):
    # Method bound to `EVT_PDU_RECV` event.
    if isinstance(event.pdu, A_RELEASE_RQ):
        self._on_handle_a_release_rq(event)

def _on_handle_a_release_rq(self, event):
    try:
        do_something_with_event(event)
    except Exception:
        # The following line of code kills the Association thread. The DUL's 
        # FSM is stuck in `Sta6`, thus it is not killed by the Association 
        # thread's `kill()` method, which is invoked by the calling to `abort()` 
        # below. As a result, the `kill()` method is stuck in a `while` loop
        # trying to stop the DUL.
        event.assoc.abort()
        # Due to the above, the line below is never executed.
        do_something_else()

The above seems like a straight-forward way of aborting, but leads to leaking of DUL threads.

I have found that the below alternative works better:

def _on_pdu_recv(self, event):
    # Method bound to `EVT_PDU_RECV` event.
    if isinstance(event.pdu, A_RELEASE_RQ):
        self._on_handle_a_release_rq(event)

def _on_handle_a_release_rq(self, event):
    try:
        do_something_with_event(event)
    except Exception:
        # Manually create the PDU to abort.
        abort_pdu = A_ABORT_RQ()
        abort_pdu.source = 0x02
        abort_pdu.reason_diagnostic = 0x00
        event.assoc.dul.socket.send(abort_pdu.encode())
        event.assoc.dul.assoc.is_aborted = True
        event.assoc.dul.assoc.is_established = False
        # Hard shutdown of the Association and DUL reactors.
        event.assoc.dul.assoc._kill = True
        event.assoc.dul._kill_thread = True
        # The line below is called.
        do_something_else()

The above approach is based on this.

I am wondering what is actually the best approach to abort an association at any time. Thoughts?

[pchristos]

ping @scaramallion

[pchristos]

ping @scaramallion any take on this?