Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when CGO_ENABLED=1 while using WebviewBrowserPath option. #1569

Closed
fire988 opened this issue Jul 15, 2022 · 18 comments · Fixed by #1790
Closed

Crash when CGO_ENABLED=1 while using WebviewBrowserPath option. #1569

fire988 opened this issue Jul 15, 2022 · 18 comments · Fixed by #1790
Labels
awaiting feedback More information is required from the requestor Bug Something isn't working
Milestone

Comments

@fire988
Copy link

fire988 commented Jul 15, 2022

Description

when CGO_ENABLED set to 1, and load webview in custom location using WebviewBrowserPath, the output binary executable file is buggy, It will crash at GetAvailableCoreWebView2BrowserVersionString function.

To Reproduce

none

Expected behaviour

when CGO_ENABLED set to 1, and load webview in custom location using WebviewBrowserPath, the output binary executable file is buggy, It will crash at GetAvailableCoreWebView2BrowserVersionString function.

Screenshots

none

Attempted Fixes

No response

System Details

Wails CLI v2.0.0-beta.38


Scanning system - Please wait (this may take a long time)...Done.

System
------
OS:             Windows 10 Pro
Version:        2009 (Build: 19044)
ID:             21H2
Go Version:     go1.17.11
Platform:       windows
Architecture:   amd64

Wails
------
Version:        v2.0.0-beta.38

Dependency      Package Name    Status          Version
----------      ------------    ------          -------
WebView2        N/A             Available
npm             N/A             Installed       8.1.2
*upx            N/A             Installed       upx 3.96
*nsis           N/A             Available

* - Optional Dependency

Diagnosis
---------
Your system has missing dependencies!

Required package(s) installation details:
  - WebView2 : Available at https://developer.microsoft.com/en-us/microsoft-edge/webview2/

Optional package(s) installation details:
  - nsis : Available at https://nsis.sourceforge.io/Download

Additional context

No response

@leaanthony
Copy link
Member

Thanks for opening this. Please provide minimal steps to reproduce. Thanks 🙏

@fire988
Copy link
Author

fire988 commented Jul 15, 2022

STEPS:
under windows.

  1. wails init a hello world project.

  2. modify the main function in main.go file,like below:
    func main() {
    // Create an instance of the app structure
    app := NewApp()

    // Create application with options
    err := wails.Run(&options.App{
    Title: "webview-test",
    Width: 1024,
    Height: 768,
    DisableResize: false,
    Fullscreen: false,
    Frameless: false,
    StartHidden: false,
    HideWindowOnClose: false,
    RGBA: &options.RGBA{255, 255, 255, 255},
    Assets: assets,
    LogLevel: logger.DEBUG,
    OnStartup: app.startup,
    OnDomReady: app.domReady,
    OnShutdown: app.shutdown,
    Bind: []interface{}{
    app,
    },
    // Windows platform specific options
    Windows: &windows.Options{
    WebviewIsTransparent: false,
    WindowIsTranslucent: false,
    DisableWindowIcon: false,
    WebviewBrowserPath: "path/to/webview2/",
    },
    })

    if err != nil {
    log.Fatal(err)
    }
    }

  3. Set enviroment variable CGO_ENABLED to 1

  4. wails build

  5. run it

@stffabi
Copy link
Collaborator

stffabi commented Jul 15, 2022

Would it be possible for you to also post the stacktrace of the panic?

You need to compile the binary with 'wails build --debug' to get the stacktrace shown in the console. That would be awesome 🙏

@NanoNik
Copy link
Contributor

NanoNik commented Jul 15, 2022

This is an issue in manual mapping of WebView2Loader.dll, probably something going wrong with TLS, as it is used inside GetAvailableCoreWebView2BrowserVersionString, if library loaded from disk, everything works fine. (This happens only with GetAvailableCoreWebView2BrowserVersionString)

@leaanthony
Copy link
Member

I'm thinking we might need more docs around this feature in the guide section? Thoughts?

@stffabi
Copy link
Collaborator

stffabi commented Jul 15, 2022

I wonder if it is related to the wrong handling of unsafe.Pointer to uint64 of the webview2loader when loading from the embedded version, e.g here

uint64(uintptr(unsafe.Pointer(browserPath))),

Normally native calls use uintptr with the uintptrescape compiler directive and not uint64 and maybe activating cgo has some side effects which trigger a stackgrow or something else.

@NanoNik do you have a stack trace of the panic?

I might find some time to investigate it over the weekend.

@NanoNik
Copy link
Contributor

NanoNik commented Jul 15, 2022

@stffabi No panics, function just returns ERROR_SUCCESS. Actually, I met this issue without using CGO, but it was on Windows 7.

@fire988
Copy link
Author

fire988 commented Jul 16, 2022

Sorry, the information I provided was incorrect and is corrected as follows:

A cgo.go file needs to be created in the root of the project with the following contents:
package main

/*
#include <stdio.h>

int printint(int v) {
printf("printint: %d\n", v);
if (v==1)
return 1;
return v+printint(v-1);
}
*/
import "C"

func ctest() {
v := 100
n := C.printint(C.int(v))
println(n)
}

Then the bug can be triggered, the output after adding the --debug compile option is as follows:

---- WARNING ----
2022/07/16 13:22:04 Unable to call GetAvailableCoreWebView2BrowserVersionString (80070032): The operation completed successfully.

the program terminated with no panic.

@stffabi
Copy link
Collaborator

stffabi commented Jul 16, 2022

The error code implies that something is not supported. There's one issue on the webview2feedback repo which also mentions this error code when using a fixed version deployment and a relative path to the runtime MicrosoftEdge/WebView2Feedback#2025

Are you using a relative path?

Just to make sure I understand you correctly, the very same code with 'CGO_ENABLED=0' work just fine?

@leaanthony
Copy link
Member

I've asked Champnic for an update on the linked bug in Webview2 👍

@fire988
Copy link
Author

fire988 commented Jul 16, 2022

The error code implies that something is not supported. There's one issue on the webview2feedback repo which also mentions this error code when using a fixed version deployment and a relative path to the runtime MicrosoftEdge/WebView2Feedback#2025

Are you using a relative path?

Just to make sure I understand you correctly, the very same code with 'CGO_ENABLED=0' work just fine?

This problem has nothing to do with whether relative paths are used or not.

As long as the C code is not called, there is no problem.

I have done the following test:

  1. create a .go file and set it's content only two lines:
    package main
    import "C"
  2. set CGO_ENABLED to 1
  3. the bug triggered.
  4. set CGO_ENABLED to 0
  5. It works fine.

@stffabi
Copy link
Collaborator

stffabi commented Jul 16, 2022

@fire988 Thanks for clarifying 🙏. I'll take a deeper look into it over the weekend.

@leaanthony
Copy link
Member

Yeah, I'm not sure why Windows returns ERROR_SUCCESS for that method when clearly it's a "Not Supported" error. Very weird.

@stffabi
Copy link
Collaborator

stffabi commented Jul 17, 2022

I was able to reproduce it and it really appears as soon as cgo is used.
I can also confirm what @NanoNik stated, it only manifests when loading the Webview2Loader.dll from memory. Loading it from disk works as expected (using the normal loading process).It's also only a problem for getting the Version String function, skipping it and just try to load the Webview2 works also.

So it's some side effect of cgo in combination with the memory loader https://github.com/jchv/go-winloader

Maybe @jchv has an idea what is going on.

@leaanthony leaanthony added Bug Something isn't working and removed Under Investigation labels Jul 21, 2022
@leaanthony leaanthony moved this to Todo in Known Bugs Jul 21, 2022
@leaanthony leaanthony added this to the v2.0.0 milestone Aug 20, 2022
stffabi added a commit to stffabi/wails that referenced this issue Aug 25, 2022
…xed runtime

This fixes a problem with the go-winloader and using GetAvailableCoreWebView2BrowserVersionString

Fixes wailsapp#1569
@stffabi
Copy link
Collaborator

stffabi commented Aug 25, 2022

There's a draft PR #1790 up for fixing this issue. Would it be possible for you @fire988 to give it a try? That would be awesome.

@stffabi stffabi added the awaiting feedback More information is required from the requestor label Aug 25, 2022
leaanthony added a commit that referenced this issue Aug 26, 2022
…xed runtime (#1790)

* [webview2loader] Start porting of OpenWebView2Loader to go

* [webviewloader] Use go implementation to retrieve the version of a fixed runtime

This fixes a problem with the go-winloader and using GetAvailableCoreWebView2BrowserVersionString

Fixes #1569

Co-authored-by: Lea Anthony <[email protected]>
Repository owner moved this from Todo to Done in Known Bugs Aug 26, 2022
@leaanthony
Copy link
Member

@fire988 Master now has the fixes for this. Please test using this guide. If it doesn't work as expected, please reopen 👍

@fire988
Copy link
Author

fire988 commented Aug 27, 2022

I've done a full test and the problem has been solved, great!

@stffabi
Copy link
Collaborator

stffabi commented Aug 27, 2022

Awesome thanks for taking your time and testing it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback More information is required from the requestor Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants