-
Notifications
You must be signed in to change notification settings - Fork 263
Enhance synctrace logging #980
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
base: dev
Are you sure you want to change the base?
Conversation
Switched the maps to be indexed by the .Pointer (not a string) Grouped the lockCount, unlockCount ,and lastLock in an trackingEntry so we can detect unlocks of something that wasn't ever locked and excessive unlocks and also tracks the first time locked and the last unlock time. Added LogDangledLocks for debugging use. Added a panic handler to the Main so we can log out panics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the synctrace logging functionality for Mutex and RWMutex operations to improve debugging of concurrent operations. The implementation switches from string-based indexing to pointer-based indexing for better performance and adds detection of lock/unlock imbalances.
Key changes:
- Refactored tracking maps to use
uintptrkeys instead of string keys for improved performance - Added comprehensive tracking with
trackingEntrystruct including first lock, last lock, and last unlock times - Implemented
LogDangledLocks()function to detect held locks and excessive unlocks
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/sync/log.go | Core refactoring: switched to uintptr-based tracking, added trackingEntry struct, implemented LogDangledLocks and unlock validation |
| internal/sync/release.go | Added no-op LogDangledLocks for non-synctrace builds |
| main.go | Added panic recovery handler with logging |
| webrtc.go, web_tls.go, usb_mass_storage.go, native.go, etc. | Migrated from stdlib sync to internal/sync package |
| DEVELOPMENT.md | Added documentation for synctrace debugging and troubleshooting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
This update enhances the performance and features of the
synctracelogging ofLock/Unlock/TryLockforMutexandRWMutexfor improved visibility into concurrent operations and aids in debugging synchronization-related issues by providing more detailedsynctracelogging.sync changes
.Pointeras auintptr(not as astring) somapis smaller and faster.lockCount,unlockCount, andlastLockinto atrackingEntryso we can detect unlocks of something that wasn't ever locked, excessive unlocks, and also track thefirstLockand thelastUnlock.LogDangledLocksfor debugging use, just insert a call to this anywhere you might have concerns.other changes
Migrates from standard library
syncpackage imports to an internalinternal/syncpackage across almost all of the codebase.Exceptions:
sync.Oncewhich isn't worth trackingcheckIPfunction to sync its own children*These are performance sensitive or very frequent lock/unlock that just emits too many logs to track. If there's a problem in those modules, you can temporarily swap over to using the logger by replacingimport "sync"withimport "github.com/jetkvm/kvm/internal/sync"