atmosphere icon indicating copy to clipboard operation
atmosphere copied to clipboard

Broadcasters with names that contain both { and } are never cleaned up

Open alexkau opened this issue 1 year ago • 1 comments

Describe the bug Broadcasters with names that contain both { and } are never cleaned up.

Atmosphere Info 2.7.4 (bug introduced in 2.6.0)

Repro Steps Configure a lifecycle strategy, such as IDLE_EMPTY_DESTROY, which should cause the broadcaster to automatically close at some point. Create a broadcaster that uses both { and } in its name, and one that does not. Disconnect from the broadcasters and wait until they should be destroyed.

Expected behavior Both broadcasters are destroyed.

Actual results The broadcaster with { and } in its name is never destroyed.

Additional context This was introduced by the fix for #2407, and is coming from the first few lines in LifecycleHander: https://github.com/Atmosphere/atmosphere/commit/b5d4c9ac269235db4c55ee882b9e1d530858db6b#diff-bdbb29403dc693807ab47591ccfa632a3790cc0751cf72b0ece5082b3eb22e19

        if (broadcaster.getID().contains("{") && broadcaster.getID().contains("}")) {
            logger.trace("Ignoring wildcard {} with lifecycle policy: {}", broadcaster.getID(), lifeCyclePolicy.getLifeCyclePolicy().name());
            return this;
        }

I'm not able to find any mention of this change in the wiki, and cannot find any release notes for 2.6 at all, so this undocumented change has caused my application to build up an excessive number of broadcasters for about the last year since we upgraded to 2.7.

We're implementing a workaround to avoid putting { and } in our broadcaster names, but I wanted to file this anyways since this limitation -- if it is intentional -- should really be documented.

alexkau avatar Feb 10 '23 22:02 alexkau

@alexkau Nice catch. Clearly a regression I've introduced. Will take a look once I have cycle, or improve the documentation.

jfarcand avatar Feb 13 '23 15:02 jfarcand