Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Register
  • Sign in
  • L libnice
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 55
    • Issues 55
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 13
    • Merge requests 13
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • libnice
  • libnice
  • Issues
  • #110
Closed
Open
Issue created Apr 28, 2020 by Juan Navarro@j1eloContributor

nice_agent_remove_stream() was silently made asynchronous

nice_agent_remove_stream() has always been a synchronous function, but commit 0556ec49 introduced a non-backwards compatible change that can break user applications, by making this function asynchronous.

Net effect of the breakage is that an application was correctly disposing NiceAgent and closing all ICE sockets before libnice 0.1.16, and was leaking all sockets + Agent memory after 0.1.16. This happens because previously it was possible to run this function without a main loop / context (e.g. during a destruction phase where the NiceAgent and all its relevant resources are being immediately shut down). The new behavior is that nice_agent_remove_stream() leaks a dangling Agent reference if on_stream_refreshes_pruned() never gets to run (async).

(an aggravating issue is that this behavior will be totally unnoticed, unless explicitly observing FDs with lsof, or for applications that manage to end up exhausting all available system sockets)

My proposed solution is to revert this change, while the new async functionality could have been introduced in a new method nice_agent_remove_stream_async(), such as proposed in #13

Alternatively, this change (and similar others, if more changes like this are being made) should be accompanied with neon flashing lights in the release notes, documentation, and even runtime warnings, because otherwise key semantic changes will pass unnoticed to users of the library.

Assignee
Assign to
Time tracking