Fix two race conditions and a type conversion mistake #70

GeertJohan · 2019-02-16T18:26:25Z

I have not tested these changes, please test before merging
Sadly I cannot test due to #68
The code compiles and I've looked over the changes 5 times. But please test the changes before merging!

This PR fixes two race conditions which may occur when multiple engines are created at the same time.

The first concerns access around the global slice flutterEngines. When appended to from multiple goroutines simultaniously, one append may be overwritten by the other.
The second concerns the obtaining of index in gutter.go:235. Because the len() and append() operations to the flutterEngines slice are not occurring in a locked transaction, and because the index is appointed before the slice is actually grown. Two goroutines may read len() to be 0, which they use as the index by settings glfw window userdata. Then both engines are started (.Run()). Only at that time are the engines added to the flutterEngines slice, causing it to growing two times. The first engine gets the correct index in the slice (0), the second engine gets index 1 in the slice. The GLFW window for the second engine has index 0 in the user data. Chaos occurs. 😄

Then, I also found some type inconsistencies. The GLFW window userdata is set to a pointer to a go integer. But when read they are cast to *C.int. I'm not sure if it breaks anything, but it seems wrong, so I've change the types.

Perhaps one of these changes is related to #32 ?

pchampio · 2019-02-16T18:55:14Z

Awesome! I will test the changes later, but it looks fine.

pchampio

💯 working

Support Chinese URL (go-flutter-desktop#48) Fixes go-flutter-desktop#48 Support MacShortcuts Support quick cursor move Clarification of the purpose explanation. go-flutter-desktop#54 Fixes go-flutter-desktop#54, confusing README Update README.md Fix regression caused by go-flutter-desktop#47 Feature/keyboard layout support (go-flutter-desktop#57) * Support different layout * Exported struct, user can use it in main * Renaming function (matchin new format) * Added default value for shortcuts * Removed local import (testing) * Added keyboard config * Check for optionnal keyboard * Fixed renaming * Fixed regression * new example for keyboard * Added explaination Now only send boolean Changed to boolean parameter & fixed textSelection Fixed some issue, added backspace wordTravellerKEy support wordTravellerKEy added (alt for MacOS, Ctrl for others) Update BUG.md (go-flutter-desktop#69) Removes the "task list" from the template. Add gl_proc_resolver to resolve GL procs within the GLFW context (go-flutter-desktop#71) Fix two race conditions and a type conversion mistake (go-flutter-desktop#70) GLFW callbacks: Obtaining the index of the engine using the window userPointer Engine Args: main_path and packages_path should be set null Assert that EngineOpenGL was created using NewEngineOpenGL (go-flutter-desktop#73) Update LICENSE Calculate pixel_ratio based on users' display (go-flutter-desktop#75) Update release mode flag Example: User defined Pixel Ratio Moved project (go-flutter-desktop#76) Rename a bunch of things: - import path github.com/Drakirus/go-flutter-desktop-embedder > github.com/go-flutter-desktop/go-flutter - package path go-flutter/flutter > go-flutter/embedder - package name flutter > embedder - package name gutter > flutter - embedder.EngineOpenGL > embedder.FlutterEngine - identifier flutterOGL > flutterEngine - formatted embedder.embedder_helper.c README.md: Package rename, change to introduction Fix Linux control issue Fixes go-flutter-desktop#79: cgo identifiers conflict README.md: Add version compatibility information Deprecate OptionWindowInitializer, add WindowIcon Update embedder definitions (go-flutter-desktop#81) updated stocks demo so it work with flutter 1.2.1 Update README.md (go-flutter-desktop#85) * Update README.md fixes cd path Add go.mod Fix go.mod Add codecs and a plugin structure, rework the system plugins. chore: align method receivers Add debugging values in dpi calculation README.md: Add godoc and go report card badge plugin: Add tests plugin: Add json method codec tests plugin: Add first method channel tests chore: Make lint and vet happier and fix spelling mistakes. chore: Improve identifiers .github: Update and simplify the issue template .github: Add template for plugin issues fixes: gitter link in README.md chore: adds link to plugins repo godoc: Handle doc 'MissingPluginException' Add sane defaults for assets and icudtl Remove forced pixelratio Oops, debugging leftover.. Resolve executable path for flutter_assets an icudtl.dat internal/execpath: cleanup Improve engine result handling Add support for sync method handlers Remove hover desktop setup from example go.mod: Add test dependencies Remove duplicate pixels to screencoordinates calculation Fixes go-flutter-desktop#101 .github: Add example template Fix broken tests, now using ResponseSender. README.md: Restructure and simpler getting started Remove examples Fix panic and remove glfw dependency from embedder chore: cleanup and deprecation warnings Upgrade embedder.h, implement mouse hovering, cleanup glfw cursor handling embedder: Add README.md embedder/README.md: cleanup and typo's Cleanup PointerPhaseNone Cleanup debug remnant README.md: Add mouse-over to supported features Fix go-flutter-desktop#113: Don't double-add event of pointerPhase embedder: upgrade embedder.h to flutter/engine for v1.5.4-hotfix.2 (52c7a1e849a170be4b2b2fe34142ca2c0a6fea1f) Implement pointer scrolling. Fixes go-flutter-desktop#6 Add initial implementation for system navigator pop event (go-flutter-desktop#142) Unexport PopBehaviorKind chore: Remove relic of the past Add WindowMode Option for bordreless and borderless fullscreen windows. (go-flutter-desktop#144) Add window dimension limits (go-flutter-desktop#145) * Add window dimensions limits Add renovate.json Update github.com/go-gl/glfw commit hash to e6da0ac pull origin Revert "pull origin" This reverts commit 7f1705a. feature: support for RawKeyboard events Uses the the RawKeyEventDataLinux for every platform. RawKeyEventDataLinux is the only Flutter implementation that support the GLFW Toolkit, the events data is sended as it is, no translation is made in go-flutter, everything is handled by the Flutter framework. Update README.md translate glfw.Repeat to glfw.Press to match other shell's keyevents fix comment Removed debug Removed debug Renamed var (coded = ☁️ or 😴 New way to handle keyboard New way to handle keyboard shortcut bind Fixed masking issue Fixed masking

Fix two race conditions and a type conversion mistake

1729e95

GeertJohan force-pushed the feature/fix-race-condition branch from 5bc54ab to 1729e95 Compare February 16, 2019 18:32

pchampio self-requested a review February 16, 2019 18:41

pchampio approved these changes Feb 17, 2019

View reviewed changes

pchampio merged commit 686b0b7 into go-flutter-desktop:master Feb 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix two race conditions and a type conversion mistake #70

Fix two race conditions and a type conversion mistake #70

GeertJohan commented Feb 16, 2019 •

edited

Loading

pchampio commented Feb 16, 2019

pchampio left a comment

Fix two race conditions and a type conversion mistake #70

Fix two race conditions and a type conversion mistake #70

Conversation

GeertJohan commented Feb 16, 2019 • edited Loading

pchampio commented Feb 16, 2019

pchampio left a comment

Choose a reason for hiding this comment

GeertJohan commented Feb 16, 2019 •

edited

Loading