Skip to content

fix(windows): Refactor static globals to instance members#1970

Merged
Gustl22 merged 3 commits into
bluefireteam:mainfrom
idcpj:main
May 26, 2026
Merged

fix(windows): Refactor static globals to instance members#1970
Gustl22 merged 3 commits into
bluefireteam:mainfrom
idcpj:main

Conversation

@idcpj

@idcpj idcpj commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

fix(windows): support multi-engine and fix crashes in multi-window environments

Description

Previously, AudioplayersWindowsPlugin utilized static inline global variables for the MethodChannel and EventStreamHandler. In multi-window scenarios (e.g., using desktop_multi_window), where multiple Flutter Engines are initialized, these static pointers were overwritten by the most recently opened window. This caused the plugin state of previous windows to become corrupted, leading to crashes and inconsistent behavior.

This PR refactors the plugin to move these globals into instance members. This ensures:

  • Each Engine instance maintains its own independent MethodChannel and EventStreamHandler.
  • Plugin states are completely isolated across different windows.
  • Memory safety is improved by preventing the overwriting of global static pointers.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, refactor:, docs:, chore:, test:, ci: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality. (N/A for architectural refactor)
  • I have updated/added relevant documentation and added dartdoc comments with ///, where necessary.
  • I have updated/added relevant examples in example.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

Resolves crashes in multi-window/multi-engine environments on Windows.

…multiple windows

- Replaced all `static inline` global variables with instance members in `AudioplayersWindowsPlugin`.
- Ensured each Engine instance possesses its own independent `MethodChannel` and `EventStreamHandler`.
- Fixed crashes occurring when opening a second window due to global static pointer overwriting.
- Isolated plugin state across different windows to prevent interference.
playerEventChannels;
// Map to keep player event handlers alive
std::map<std::string, std::unique_ptr<EventStreamHandler<EncodableValue>>>
playerEventHandlers;

@Gustl22 Gustl22 May 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable actually is never filled with handlers. But it might not be needed in the first place?


// Keep the event channel and handler alive as long as the plugin/player
// exists
playerEventChannels[playerId] = std::move(eventChannel);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is actually necessary to hold the values in an extra array. Theoretically the channels should be referenced by the AudioPlayer instance and thus in the audioPlayers map / array. So they should be kept alive as long as the players are present in the array, or am I mistaken?

auto handler = std::make_unique<EventStreamHandler<EncodableValue>>();
plugin->globalEvents = handler.get();

plugin->globalEventChannel->SetStreamHandler(std::move(handler));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: do you think it would make sense to have a constructor taking all the initial parameters, similar to Flutter plugins:
https://github.com/flutter/packages/blob/a0ad9d99b16e1879f597f9c0b0150cb01f29a57e/packages/camera/camera_windows/windows/camera_plugin.cpp#L122

@Gustl22

Gustl22 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Sorry for the big delay. Thank you for the contribution! A few questions / proposals still, if you have a moment :)

@Gustl22 Gustl22 changed the title fix(windows): refactor static globals to instance members to support … fix(windows): Refactor static globals to instance members May 26, 2026

@Gustl22 Gustl22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@idcpj @spydon I pushed to remove the unnecessary variable 426ea3c. The other changes LGTM / make it more error safe IMO.

@Gustl22 Gustl22 merged commit 992d507 into bluefireteam:main May 26, 2026
23 of 28 checks passed
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.

3 participants