Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible memory leak when manually aborting an association #912

Open
pchristos opened this issue Jan 30, 2024 · 4 comments
Open

Possible memory leak when manually aborting an association #912

pchristos opened this issue Jan 30, 2024 · 4 comments
Labels
Milestone

Comments

@pchristos
Copy link

pchristos commented Jan 30, 2024

Hi,

I have investigated a memory leak, which seems to be related to attempts of manually aborting associations.

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?

Originally posted by @pchristos in #652 (comment)

pydicom 2.3.1
pynetdicom 2.0.2

@scaramallion scaramallion added this to the v2.1.0 milestone Jan 30, 2024
@scaramallion
Copy link
Member

Yeah, there's some iffy behaviour in shutting the DUL down properly that will be fixed in the next release.

@pchristos
Copy link
Author

Thanks! ETA for the next release?

@scaramallion
Copy link
Member

It'll be quite a while, I think. I've been focussing on pydicom's v3.0 release.

@pchristos
Copy link
Author

In that case, what's your recommended approach? Is the proposed alternative a valid approach for now?

@scaramallion scaramallion modified the milestones: v2.1.0, v2.2.0 May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants