Bug report
Describe the bug
On Windows, cloud_firestore stores its flutter::BinaryMessenger* as a
class-level static and reassigns it on every RegisterWithRegistrar:
// windows/cloud_firestore_plugin.h
static flutter::BinaryMessenger* messenger_;
// windows/cloud_firestore_plugin.cpp, RegisterWithRegistrar
messenger_ = registrar->messenger();
Every event channel is then built from that one static:
// RegisterEventChannel / RegisterEventChannelWithUUID
event_channels_[channelName] =
std::make_unique<flutter::EventChannel<flutter::EncodableValue>>(
CloudFirestorePlugin::messenger_, channelName, ...);
A Flutter Windows app can host several engines in one process (for example via
desktop_multi_window), and the standard pattern for that is to register
plugins for each engine as it is created:
DesktopMultiWindowSetWindowCreatedCallback([](void *controller) {
auto *view_controller =
reinterpret_cast<flutter::FlutterViewController *>(controller);
RegisterPlugins(view_controller->engine());
});
That is required for other plugins (window_manager fails without it), but it
means the last engine to register owns the messenger for the whole process.
From that point on:
- a snapshot listener opened from any engine is built on the last engine's
messenger, so its events are delivered to the wrong Dart isolate and the
caller silently never receives them; and
- once that engine is destroyed, the pointer dangles and the next snapshot call
crashes.
Note the first symptom is silent. The second is a hard crash.
Steps to reproduce
- A Flutter Windows app using
cloud_firestore and desktop_multi_window.
- In
windows/runner/main.cpp, register plugins for created windows as above.
- Sign in and open a screen that has a
DocumentReference.snapshots()
listener. It works.
- Create a second window (a second engine). It registers the plugins, taking
over messenger_.
- From the first window, open another screen that starts a
DocumentReference.snapshots() listener.
Expected behavior
Each engine's snapshots use that engine's own messenger; a second engine has no
effect on the first.
Actual behavior
Access violation. Captured with cdb, symbols from a RelWithDebInfo-style
build:
(46ac.45bc): Access violation - code c0000005 (!!! second chance !!!)
relay!flutter::EventChannel<flutter::EncodableValue>::SetStreamHandler+0x198
relay!cloud_firestore_windows::RegisterEventChannel+0x224
relay!cloud_firestore_windows::CloudFirestorePlugin::DocumentReferenceSnapshot+0x425
...
USER32!UserCallWinProcCheckWow
USER32!DispatchMessageWorker
relay!wWinMain+0x22b
Query snapshots take a different path and appear to work for longer, which
makes this look at first like a bug in whichever screen happens to use
DocumentReference.snapshots().
Suggested fix
Make the messenger per plugin instance rather than static, and pass it to the
channel builders. In our tree this was about 30 lines:
messenger_ loses static and becomes a member;
RegisterWithRegistrar assigns plugin->messenger_ before
registrar->AddPlugin(std::move(plugin));
RegisterEventChannel / RegisterEventChannelWithUUID take a
flutter::BinaryMessenger*;
- the five call sites pass
messenger_ — all are already inside member
functions, so nothing else needed restructuring.
We audited the plugin's other shared state and believe it does not need
changing:
| State |
Keyed by |
Verdict |
event_channels_ |
UUID-suffixed channel name |
no cross-engine collision |
stream_handlers_ |
same UUID |
no collision |
transaction_handlers_ |
transaction id |
no collision |
transactions_ |
transaction id |
no collision |
firestoreInstances_ |
app + database |
correct to share; one native Firestore per process |
messenger_ was the only one that had to be per-registration.
We ran the patch successfully against our app before reverting it for reasons
unrelated to correctness (we did not want to pin the package across all
platforms to carry a Windows-only patch).
Environment
cloud_firestore 6.7.1 (the code is the same in current main at time of
writing)
- Flutter 3.44.6, Windows 11 (26200), x64, release build
desktop_multi_window 0.3.0
Bug report
Describe the bug
On Windows,
cloud_firestorestores itsflutter::BinaryMessenger*as aclass-level static and reassigns it on every
RegisterWithRegistrar:Every event channel is then built from that one static:
// RegisterEventChannel / RegisterEventChannelWithUUID event_channels_[channelName] = std::make_unique<flutter::EventChannel<flutter::EncodableValue>>( CloudFirestorePlugin::messenger_, channelName, ...);A Flutter Windows app can host several engines in one process (for example via
desktop_multi_window), and the standard pattern for that is to registerplugins for each engine as it is created:
That is required for other plugins (
window_managerfails without it), but itmeans the last engine to register owns the messenger for the whole process.
From that point on:
messenger, so its events are delivered to the wrong Dart isolate and the
caller silently never receives them; and
crashes.
Note the first symptom is silent. The second is a hard crash.
Steps to reproduce
cloud_firestoreanddesktop_multi_window.windows/runner/main.cpp, register plugins for created windows as above.DocumentReference.snapshots()listener. It works.
over
messenger_.DocumentReference.snapshots()listener.Expected behavior
Each engine's snapshots use that engine's own messenger; a second engine has no
effect on the first.
Actual behavior
Access violation. Captured with cdb, symbols from a
RelWithDebInfo-stylebuild:
Query snapshots take a different path and appear to work for longer, which
makes this look at first like a bug in whichever screen happens to use
DocumentReference.snapshots().Suggested fix
Make the messenger per plugin instance rather than static, and pass it to the
channel builders. In our tree this was about 30 lines:
messenger_losesstaticand becomes a member;RegisterWithRegistrarassignsplugin->messenger_beforeregistrar->AddPlugin(std::move(plugin));RegisterEventChannel/RegisterEventChannelWithUUIDtake aflutter::BinaryMessenger*;messenger_— all are already inside memberfunctions, so nothing else needed restructuring.
We audited the plugin's other shared state and believe it does not need
changing:
event_channels_stream_handlers_transaction_handlers_transactions_firestoreInstances_messenger_was the only one that had to be per-registration.We ran the patch successfully against our app before reverting it for reasons
unrelated to correctness (we did not want to pin the package across all
platforms to carry a Windows-only patch).
Environment
cloud_firestore6.7.1 (the code is the same in currentmainat time ofwriting)
desktop_multi_window0.3.0