fix: Address code review issues and fix miner start deadlock

- Remove deprecated GetDB() function that exposed raw DB pointer
- Fix GetLatestHashrate to distinguish sql.ErrNoRows from real errors
- Document async saves in PeerRegistry mutation methods
- Fix deadlock in XMRig/TTMiner Start() by moving CheckInstallation
  call before acquiring the main lock (Go RWMutex doesn't allow
  recursive locking)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
snider 2025-12-31 03:51:05 +00:00
parent 0e810438cd
commit 0735445eb0
5 changed files with 33 additions and 27 deletions

View file

@ -106,18 +106,6 @@ func IsInitialized() bool {
return db != nil
}
// GetDB returns the database connection (for advanced queries).
//
// Deprecated: This function is unsafe for concurrent use because the returned
// pointer may become invalid if Close() is called by another goroutine after
// GetDB() returns. Use the dedicated query functions (InsertHashratePoint,
// GetHashrateHistory, etc.) instead, which handle locking internally.
func GetDB() *sql.DB {
dbMu.RLock()
defer dbMu.RUnlock()
return db
}
// createTables creates all required database tables
func createTables() error {
schema := `

View file

@ -2,6 +2,7 @@ package database
import (
"context"
"database/sql"
"fmt"
"log"
"time"
@ -163,7 +164,10 @@ func GetLatestHashrate(minerName string) (*HashratePoint, error) {
`, minerName).Scan(&point.Timestamp, &point.Hashrate)
if err != nil {
return nil, nil // Not found is not an error
if err == sql.ErrNoRows {
return nil, nil // No data found is not an error
}
return nil, fmt.Errorf("failed to get latest hashrate: %w", err)
}
return &point, nil

View file

@ -13,6 +13,17 @@ import (
// Start launches the TT-Miner with the given configuration.
func (m *TTMiner) Start(config *Config) error {
// Check installation BEFORE acquiring lock (CheckInstallation takes its own locks)
m.mu.RLock()
needsInstallCheck := m.MinerBinary == ""
m.mu.RUnlock()
if needsInstallCheck {
if _, err := m.CheckInstallation(); err != nil {
return err // Propagate the detailed error from CheckInstallation
}
}
m.mu.Lock()
defer m.mu.Unlock()
@ -20,13 +31,6 @@ func (m *TTMiner) Start(config *Config) error {
return errors.New("miner is already running")
}
// If the binary path isn't set, run CheckInstallation to find it.
if m.MinerBinary == "" {
if _, err := m.CheckInstallation(); err != nil {
return err // Propagate the detailed error from CheckInstallation
}
}
if m.API != nil && config.HTTPPort != 0 {
m.API.ListenPort = config.HTTPPort
} else if m.API != nil && m.API.ListenPort == 0 {

View file

@ -15,6 +15,17 @@ import (
// Start launches the XMRig miner with the specified configuration.
func (m *XMRigMiner) Start(config *Config) error {
// Check installation BEFORE acquiring lock (CheckInstallation takes its own locks)
m.mu.RLock()
needsInstallCheck := m.MinerBinary == ""
m.mu.RUnlock()
if needsInstallCheck {
if _, err := m.CheckInstallation(); err != nil {
return err // Propagate the detailed error from CheckInstallation
}
}
m.mu.Lock()
defer m.mu.Unlock()
@ -22,13 +33,6 @@ func (m *XMRigMiner) Start(config *Config) error {
return errors.New("miner is already running")
}
// If the binary path isn't set, run CheckInstallation to find it.
if m.MinerBinary == "" {
if _, err := m.CheckInstallation(); err != nil {
return err // Propagate the detailed error from CheckInstallation
}
}
if m.API != nil && config.HTTPPort != 0 {
m.API.ListenPort = config.HTTPPort
} else if m.API != nil && m.API.ListenPort == 0 {

View file

@ -91,6 +91,8 @@ func NewPeerRegistryWithPath(peersPath string) (*PeerRegistry, error) {
}
// AddPeer adds a new peer to the registry.
// Note: Persistence is debounced (writes batched every 5s). Call Close() to ensure
// all changes are flushed to disk before shutdown.
func (r *PeerRegistry) AddPeer(peer *Peer) error {
r.mu.Lock()
@ -120,6 +122,7 @@ func (r *PeerRegistry) AddPeer(peer *Peer) error {
}
// UpdatePeer updates an existing peer's information.
// Note: Persistence is debounced. Call Close() to flush before shutdown.
func (r *PeerRegistry) UpdatePeer(peer *Peer) error {
r.mu.Lock()
@ -136,6 +139,7 @@ func (r *PeerRegistry) UpdatePeer(peer *Peer) error {
}
// RemovePeer removes a peer from the registry.
// Note: Persistence is debounced. Call Close() to flush before shutdown.
func (r *PeerRegistry) RemovePeer(id string) error {
r.mu.Lock()
@ -180,6 +184,7 @@ func (r *PeerRegistry) ListPeers() []*Peer {
}
// UpdateMetrics updates a peer's performance metrics.
// Note: Persistence is debounced. Call Close() to flush before shutdown.
func (r *PeerRegistry) UpdateMetrics(id string, pingMS, geoKM float64, hops int) error {
r.mu.Lock()
@ -201,6 +206,7 @@ func (r *PeerRegistry) UpdateMetrics(id string, pingMS, geoKM float64, hops int)
}
// UpdateScore updates a peer's reliability score.
// Note: Persistence is debounced. Call Close() to flush before shutdown.
func (r *PeerRegistry) UpdateScore(id string, score float64) error {
r.mu.Lock()