jcarr
/
git2go
1
0
Fork 0
Code Issues 52 Pull Requests 31 Packages Projects Releases Wiki Activity

More diff functionality #629

Merged
mbfr merged 24 commits from feature-diff-extra into master 2020-08-18 08:14:03 -05:00
Conversation 0 Commits 24 Files Changed 3 -3
1 changed files with 0 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 97267d3f6e - Show all commits

3
diff.go
View File

@ -912,9 +912,6 @@ func deltaApplyCallback(_delta *C.git_diff_delta, _payload unsafe.Pointer) C.int
func DefaultApplyOptions() (*ApplyOptions, error) {
opts := C.git_apply_options{}
runtime.LockOSThread()
defer runtime.UnlockOSThread()
C._go_git_apply_init_options(&opts)
lhchavez commented 2020-08-17 08:19:50 -05:00 (Migrated from github.com)
Review
Copy Link

Can these be returned to their old place? The reason why they are needed is that when runtime.LockOsThread()/runtime.UnlockOSThread() combo is called, it asks Go to guarantee that only this code executes on that thread (since Go is completely free to move stuff around when needed, at any point in time). That's the only way in which it can be guaranteed that if git_apply_options_init happens to place any error information in the Thread-local storage, MakeGitError() below can still find it.

We may need to tweak script/check-MakeGitError-thread-lock.go to also complain if these functions don't happen before any cgo calls in the function to make this less error-prone.

Can these be returned to their old place? The reason why they are needed is that when [`runtime.LockOsThread()`](https://golang.org/pkg/runtime/#LockOSThread)/`runtime.UnlockOSThread()` combo is called, it asks Go to guarantee that _only_ this code executes on that thread (since Go is completely free to move stuff around when needed, at any point in time). That's the only way in which it can be guaranteed that if `git_apply_options_init` happens to place any error information in the Thread-local storage, `MakeGitError()` below can still find it. We may need to tweak `script/check-MakeGitError-thread-lock.go` to _also_ complain if these functions don't happen before any cgo calls in the function to make this less error-prone.
mbfr commented 2020-08-18 02:16:34 -05:00 (Migrated from github.com)
Review
Copy Link

Fixed

Fixed
return applyOptionsFromC(&opts), nil