cogentcore / core

A free and open source framework for building powerful, fast, and elegant 2D and 3D apps that run on macOS, Windows, Linux, iOS, Android, and the Web with a single pure Go codebase, allowing you to Code Once, Run Everywhere.
http://cogentcore.org/core
BSD 3-Clause "New" or "Revised" License
1.31k stars 71 forks source link

Refactoring for concurrency security #891

Closed ddkwork closed 4 months ago

kkoreilly commented 4 months ago

I appreciate your work on this, and I will consider the possibility of merging this. However, I need to investigate the cause of the map panic first. I should have a decision for you tomorrow.

ddkwork commented 4 months ago

The Map problem is completely gone, and the next step is to get more unit tests to work

kkoreilly commented 4 months ago

I am going to try to figure out the underlying cause of several of these crashes and implement a more optimized solution. If that does not work, then I will consider merging your PR. I should be able to get back to you later today.

ddkwork commented 4 months ago

I am going to try to figure out the underlying cause of several of these crashes and implement a more optimized solution. If that does not work, then I will consider merging your PR. I should be able to get back to you later today.

OK, thanks

ddkwork commented 4 months ago

I am going to try to figure out the underlying cause of several of these crashes and implement a more optimized solution. If that does not work, then I will consider merging your PR. I should be able to get back to you later today.

Can you fix the vulkan error again, I forgot where you fixed it last time, the latest commit recurred, or async minimized or the example in goosi minimized triggers panic, thanks

ddkwork commented 4 months ago

I used goland for batch code cleanup, there are too many godoc and the like, plus I don't speak English well, I can't understand the comments 100%, which requires you to optimize. The rest of the redundant type conversions and so on have been changed in batches, and now the issue tab on goland is much cleaner. I remember there was a proposal to push the repository to the go curated list, so this cleanup was necessary, in addition to that, it needed to go through all the unit tests and github deployment tests, and finally it might be necessary to deal with a lot of spell checking and interface signatures or struct field renaming, in general, if the name is not shorthand, you can recognize the function function at a glance without looking at the comments, but naming them requires re-running the core gen command, which I am not very familiar with, and there is no further optimization.

kkoreilly commented 4 months ago

We are currently working on a major redesign of async updating that should fix the vulkan, window closing, and concurrent map access issues. I agree that it makes sense to have clear function and field names, and I am willing to consider renaming some of those things. You do not need to worry about updating the generated code, but if you want to, all you have to do is run go generate in any directory where you change the names of widget types and their fields. I will consider your changes to documentation and struct literal initialization, but I do not think that it is completely necessary to rewrite all of our documentation comments. As you can see, we already score A+ on the Go Report Card, and we will have addressed all of the complaints from Awesome Go by the time we do our v1 release.

kkoreilly commented 4 months ago

We completed an extensive redesign of our asynchronous updating process, and all of the issues that you had with the nil panics and the map concurrency should be fixed now. Can you try examples/async in the latest version of the main branch and tell me if you have any issues?

ddkwork commented 4 months ago

We completed an extensive redesign of our asynchronous updating process, and all of the issues that you had with the nil panics and the map concurrency should be fixed now. Can you try examples/async in the latest version of the main branch and tell me if you have any issues?

Sorry I fell asleep, I played with the code for too long yesterday and I don't know when I fell asleep, wait for me for 5 minutes and I'll go buy a cigarette and come back and continue testing your commit

ddkwork commented 4 months ago

We completed an extensive redesign of our asynchronous updating process, and all of the issues that you had with the nil panics and the map concurrency should be fixed now. Can you try examples/async in the latest version of the main branch and tell me if you have any issues?

vulkan error

Details

GOROOT=C:\Users\Admin\go\pkg\mod\golang.org\toolchain@v0.0.1-go1.22.0.windows-amd64 #gosetup GOPATH=C:\Users\Admin\go #gosetup C:\Users\Admin\go\pkg\mod\golang.org\toolchain@v0.0.1-go1.22.0.windows-amd64\bin\go.exe build -o C:\Users\Admin\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___go_build_cogentcore_org_core_examples_async.exe cogentcore.org/core/examples/async #gosetup C:\Users\Admin\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___go_build_cogentcore_org_core_examples_async.exe 2024/02/17 18:16:37 panic: vulkan error: vulkan error: out of host memory (-1) 2024/02/17 18:16:37 2024/02/17 18:16:37 ----- START OF STACK TRACE: ----- 2024/02/17 18:16:37 goroutine 8 [running]: runtime/debug.Stack() C:/Users/Admin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.0.windows-amd64/src/runtime/debug/stack.go:24 +0x5e cogentcore.org/core/goosi.HandleRecoverBase({0x7ff6f2d0daa0, 0xc001563c50}) C:/Users/Admin/Desktop/core-main/goosi/recover.go:49 +0x4b cogentcore.org/core/gi.HandleRecover({0x7ff6f2d0daa0, 0xc001563c50}) C:/Users/Admin/Desktop/core-main/gi/recover.go:32 +0x4b cogentcore.org/core/gi.(*RenderWin).EventLoop.func1() C:/Users/Admin/Desktop/core-main/gi/renderwin.go:546 +0x24 panic({0x7ff6f2d0daa0?, 0xc001563c50?}) C:/Users/Admin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.0.windows-amd64/src/runtime/panic.go:770 +0x132 cogentcore.org/core/vgpu.IfPanic(...) C:/Users/Admin/Desktop/core-main/vgpu/errors.go:39 cogentcore.org/core/vgpu.(*Surface).ConfigSwapchain(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:254 +0x906 cogentcore.org/core/vgpu.(*Surface).ReConfigSwapchain(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:302 +0x25 cogentcore.org/core/vgpu.(*Surface).AcquireNextImage(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:347 +0x105 cogentcore.org/core/vgpu/vdraw.(*Drawer).StartDraw(0xc000310488, 0x0) C:/Users/Admin/Desktop/core-main/vgpu/vdraw/draw.go:234 +0xa7 cogentcore.org/core/gi.(*RenderWin).DrawScenes(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:984 +0x262 cogentcore.org/core/gi.(*RenderWin).RenderWindow(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:948 +0x3b9 cogentcore.org/core/gi.(*RenderWin).HandleWindowEvents(0xc00022b0e0, {0x7ff6f3258f40, 0xc00047eb60}) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:610 +0x5ac cogentcore.org/core/gi.(*RenderWin).HandleEvent(0xc00022b0e0, {0x7ff6f3258f40, 0xc00047eb60}) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:594 +0x11c cogentcore.org/core/gi.(*RenderWin).EventLoop(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:560 +0x125 created by cogentcore.org/core/gi.(*RenderWin).GoStartEventLoop in goroutine 1 C:/Users/Admin/Desktop/core-main/gi/renderwin.go:514 +0xa5 2024/02/17 18:16:37 ----- END OF STACK TRACE ----- 2024/02/17 18:16:37 SAVED CRASH LOG TO C:\Users\Admin\AppData\Roaming\Async Updating\crash-logs\crash_2024-02-17_18-16-37 panic: vulkan error: vulkan error: out of host memory (-1) [recovered] panic: vulkan error: vulkan error: out of host memory (-1) goroutine 8 [running]: cogentcore.org/core/gi.HandleRecover({0x7ff6f2d0daa0, 0xc001563c50}) C:/Users/Admin/Desktop/core-main/gi/recover.go:69 +0x23b cogentcore.org/core/gi.(*RenderWin).EventLoop.func1() C:/Users/Admin/Desktop/core-main/gi/renderwin.go:546 +0x24 panic({0x7ff6f2d0daa0?, 0xc001563c50?}) C:/Users/Admin/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.0.windows-amd64/src/runtime/panic.go:770 +0x132 cogentcore.org/core/vgpu.IfPanic(...) C:/Users/Admin/Desktop/core-main/vgpu/errors.go:39 cogentcore.org/core/vgpu.(*Surface).ConfigSwapchain(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:254 +0x906 cogentcore.org/core/vgpu.(*Surface).ReConfigSwapchain(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:302 +0x25 cogentcore.org/core/vgpu.(*Surface).AcquireNextImage(0xc0002440b0) C:/Users/Admin/Desktop/core-main/vgpu/surface.go:347 +0x105 cogentcore.org/core/vgpu/vdraw.(*Drawer).StartDraw(0xc000310488, 0x0) C:/Users/Admin/Desktop/core-main/vgpu/vdraw/draw.go:234 +0xa7 cogentcore.org/core/gi.(*RenderWin).DrawScenes(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:984 +0x262 cogentcore.org/core/gi.(*RenderWin).RenderWindow(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:948 +0x3b9 cogentcore.org/core/gi.(*RenderWin).HandleWindowEvents(0xc00022b0e0, {0x7ff6f3258f40, 0xc00047eb60}) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:610 +0x5ac cogentcore.org/core/gi.(*RenderWin).HandleEvent(0xc00022b0e0, {0x7ff6f3258f40, 0xc00047eb60}) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:594 +0x11c cogentcore.org/core/gi.(*RenderWin).EventLoop(0xc00022b0e0) C:/Users/Admin/Desktop/core-main/gi/renderwin.go:560 +0x125 created by cogentcore.org/core/gi.(*RenderWin).GoStartEventLoop in goroutine 1 C:/Users/Admin/Desktop/core-main/gi/renderwin.go:514 +0xa5 进程 已完成,退出代码为 2

kkoreilly commented 4 months ago

When does this happen? When you close the app? Are you running the latest version of the async example?

kkoreilly commented 4 months ago

I just pushed a change that should fix the Vulkan issue and other crashes.

ddkwork commented 4 months ago

I just pushed a change that should fix the Vulkan issue and other crashes.

Change this to wait for a second, https://github.com/cogentcore/core/blob/main/examples%2Fasync%2Fasync.go#L40-L40, and then after adding more than 10 lines, you will find that the operation of the selected line is obviously delayed, which means that the rendering of the line selection cannot keep up with the speed of the mouse machine, and this problem has been appearing for a long time

ddkwork commented 4 months ago

When does this happen? When you close the app? Are you running the latest version of the async example?

yes,latest

ddkwork commented 4 months ago

I just pushed a change that should fix the Vulkan issue and other crashes.

i will try

ddkwork commented 4 months ago

I just pushed a change that should fix the Vulkan issue and other crashes.

Still panic, need debug on my machine?

kkoreilly commented 4 months ago

Yes, I will debug it on your machine.

kkoreilly commented 4 months ago

I am ready to debug it on your machine whenever you are.