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

fix missing increment of the reference count on every call to startup() #3100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbechard
Copy link

@mbechard mbechard commented Jan 3, 2025

also change how the implicit startup is tracked.
The extra change is needed since the startup() calls in
srt::CUDT::socket()
and
srt::CUDT::createGroup
would endlessly increment the startup counter as sockets were created.

This also adds handling for the case where the user code fails to call
srt_startup when initially using the library, but does later on.
It better matches up the implicit startup() with an implicit cleanup().

fixes #3098

@mbechard mbechard marked this pull request as draft January 3, 2025 22:08
@ethouris
Copy link
Collaborator

ethouris commented Jan 6, 2025

As for me it looks ok, but Max will be back in the second half of January.

@ethouris ethouris added this to the v1.5.5 milestone Jan 6, 2025
@mbechard
Copy link
Author

mbechard commented Jan 6, 2025

Still has bugs with existing code, as per the issue discussion

also change how the implicit startup is tracked.
The extra change is needed since the startup() calls in
srt::CUDT::socket()
and
srt::CUDT::createGroup
would endlessly increment the startup counter as sockets were created.

This also adds handling for the case where the user code fails to call
srt_startup when initially using the library, but does later on.
It better matches up the implicit startup() with an implicit cleanup().

fixes Haivision#3098
@mbechard mbechard marked this pull request as ready for review January 6, 2025 21:18
@mbechard
Copy link
Author

mbechard commented Jan 6, 2025

I think this modified solution is reasonable to handle the remaining issues.

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

Successfully merging this pull request may close these issues.

[BUG] Regression with commit bc2f48e057644ab9
2 participants