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 design flaw #155

Open
MayamaTakeshi opened this issue Mar 20, 2022 · 3 comments
Open

Possible design flaw #155

MayamaTakeshi opened this issue Mar 20, 2022 · 3 comments

Comments

@MayamaTakeshi
Copy link
Contributor

MayamaTakeshi commented Mar 20, 2022

I'm load testing sip.js as a UAC in this scenario (sip.js is 127.0.0.1:5040) :



             127.0.0.1:5040                127.0.0.1:5000  │
         ──────────┬─────────          ──────────┬─────────│
 08:21:17.996273   │        INVITE (SDP)         │         │
       +0.000070   │ ──────────────────────────> │         │
 08:21:17.996343   │         100 Trying          │         │
       +0.001146   │ <────────────────────────── │         │
 08:21:17.997489   │  183 Session Progress (SDP) │         │
       +0.000247   │ <────────────────────────── │         │
 08:21:17.997736   │           CANCEL            │         │
       +0.000062   │ ──────────────────────────> │         │
 08:21:17.997798   │           200 OK            │         │
                   │ <────────────────────────── │         │
                   │                             │         │
                   │                             │         │
                   │                             │         │

Notice there is no '487 Request Terminated'.
In this case, I can see memory leak as the INVITE transaction is never removed from memory as it doesn't get a final response.
So I think the handling of CANCEL should take care of starting a timer for the reception of the final response and if it is not received then generate a local '408 Request Timeout' to release the INVITE transaction from memory.

I tested and confirmed the leak with this gist:
https://gist.github.com/MayamaTakeshi/3a163c4ea4a5bf77169b1cd0337340ae

@MayamaTakeshi
Copy link
Contributor Author

MayamaTakeshi commented Mar 20, 2022

Well, considering that sip.js works purely with transactions, maybe handling of CANCEL transaction should not have any effect in the handling of the INVITE transaction.
So maybe what is missing is a way for the user of sip.js to be able to delete an existing transaction.

@kirm
Copy link
Owner

kirm commented Apr 16, 2022

I added a timer for maximum time transaction can spend in proceeding state. It is here https://github.com/kirm/sip.js/tree/Fix_%23155, please let me know if it resolves the problem.

Also I'm considering a way to add ability to kill a transaction, but that's seems to be a more complex solution than a timer.

@MayamaTakeshi
Copy link
Contributor Author

Hi,
I tested it today with both
ringTimeLimit: 15000
and
ringTimeLimit: 60000
and the problem doesn't happen anymore as the memory is properly released.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants