Compare commits

...

3 Commits

Author SHA1 Message Date
Alex P 9e95cc3f8a fix: prevent audio disconnect from blocking new WebRTC sessions
When an old connection closed while a new one started, the audio cleanup
would hold audioMutex for up to 37 seconds during CGO disconnect calls,
blocking the new session from initializing.

Use capture-clear-release pattern to minimize mutex hold time:
- Capture references to sources/relays while holding mutex
- Clear globals immediately so new sessions can proceed
- Release mutex before calling blocking Stop/Disconnect operations

This eliminates the 37-second hang during connection transitions.
2025-11-10 16:54:31 +02:00
Alex P b9705f4bac Revert "refactor: use atomic.Pointer for thread-safe inputSource access"
This reverts commit 41345b0527.
2025-11-10 15:42:10 +02:00
Adam Shiervani bc1992ea13 refactor: fix infinite useEffect 2025-11-10 09:43:10 +01:00
5 changed files with 83 additions and 63 deletions

View File

@ -205,12 +205,14 @@ lint-go-fix: build_audio_deps
# Run UI linting locally (mirrors GitHub workflow ui-lint.yml)
lint-ui:
@echo "Running UI lint..."
@cd ui && npm ci && npm run lint
@cd ui && npm ci
@cd ui && npm run lint
# Run UI linting with auto-fix
lint-ui-fix:
@echo "Running UI lint with auto-fix..."
@cd ui && npm ci && npm run lint:fix
@cd ui && npm ci
@cd ui && npm run lint:fix
# Legacy alias for UI linting (for backward compatibility)
ui-lint: lint-ui

109
audio.go
View File

@ -14,7 +14,7 @@ import (
var (
audioMutex sync.Mutex
outputSource audio.AudioSource
inputSource atomic.Pointer[audio.AudioSource]
inputSource audio.AudioSource
outputRelay *audio.OutputRelay
inputRelay *audio.InputRelay
audioInitialized bool
@ -63,15 +63,13 @@ func startAudio() error {
// Start input audio if not running, USB audio enabled, and input enabled
ensureConfigLoaded()
if inputSource.Load() == nil && audioInputEnabled.Load() && config.UsbDevices != nil && config.UsbDevices.Audio {
if inputSource == nil && audioInputEnabled.Load() && config.UsbDevices != nil && config.UsbDevices.Audio {
alsaPlaybackDevice := "hw:1,0" // USB speakers
// Create CGO audio source
newInputSource := audio.NewCgoInputSource(alsaPlaybackDevice)
var audioSrc audio.AudioSource = newInputSource
inputSource.Store(&audioSrc)
inputSource = audio.NewCgoInputSource(alsaPlaybackDevice)
inputRelay = audio.NewInputRelay(newInputSource)
inputRelay = audio.NewInputRelay(inputSource)
if err := inputRelay.Start(); err != nil {
audioLogger.Error().Err(err).Msg("Failed to start input relay")
}
@ -80,41 +78,31 @@ func startAudio() error {
return nil
}
// stopOutputLocked stops output audio (assumes mutex is held)
func stopOutputLocked() {
if outputRelay != nil {
outputRelay.Stop()
outputRelay = nil
}
if outputSource != nil {
outputSource.Disconnect()
outputSource = nil
}
}
// stopInputLocked stops input audio (assumes mutex is held)
func stopInputLocked() {
if inputRelay != nil {
inputRelay.Stop()
inputRelay = nil
}
if source := inputSource.Load(); source != nil {
(*source).Disconnect()
inputSource.Store(nil)
}
}
// stopAudioLocked stops all audio (assumes mutex is held)
func stopAudioLocked() {
stopOutputLocked()
stopInputLocked()
}
// stopAudio stops all audio
func stopAudio() {
audioMutex.Lock()
defer audioMutex.Unlock()
stopAudioLocked()
outRelay := outputRelay
outSource := outputSource
inRelay := inputRelay
inSource := inputSource
outputRelay = nil
outputSource = nil
inputRelay = nil
inputSource = nil
audioMutex.Unlock()
// Disconnect outside mutex to avoid blocking new sessions during CGO calls
if outRelay != nil {
outRelay.Stop()
}
if outSource != nil {
outSource.Disconnect()
}
if inRelay != nil {
inRelay.Stop()
}
if inSource != nil {
inSource.Disconnect()
}
}
func onWebRTCConnect() {
@ -171,8 +159,18 @@ func SetAudioOutputEnabled(enabled bool) error {
}
} else {
audioMutex.Lock()
defer audioMutex.Unlock()
stopOutputLocked()
outRelay := outputRelay
outSource := outputSource
outputRelay = nil
outputSource = nil
audioMutex.Unlock()
if outRelay != nil {
outRelay.Stop()
}
if outSource != nil {
outSource.Disconnect()
}
}
return nil
@ -190,8 +188,18 @@ func SetAudioInputEnabled(enabled bool) error {
}
} else {
audioMutex.Lock()
defer audioMutex.Unlock()
stopInputLocked()
inRelay := inputRelay
inSource := inputSource
inputRelay = nil
inputSource = nil
audioMutex.Unlock()
if inRelay != nil {
inRelay.Stop()
}
if inSource != nil {
inSource.Disconnect()
}
}
return nil
@ -240,22 +248,23 @@ func handleInputTrackForSession(track *webrtc.TrackRemote) {
continue // Drop frame but keep reading
}
// Get source atomically (hot path optimization)
source := inputSource.Load()
// Get source in single mutex operation (hot path optimization)
audioMutex.Lock()
source := inputSource
audioMutex.Unlock()
if source == nil {
continue // No relay, drop frame but keep reading
}
if !(*source).IsConnected() {
if err := (*source).Connect(); err != nil {
if !source.IsConnected() {
if err := source.Connect(); err != nil {
continue
}
}
if err := (*source).WriteMessage(0, opusData); err != nil {
(*source).Disconnect()
audioLogger.Warn().Err(err).Str("track_id", myTrackID).Msg("failed to write audio message")
if err := source.WriteMessage(0, opusData); err != nil {
source.Disconnect()
}
}
}

View File

@ -894,11 +894,20 @@ func rpcGetUsbDevices() (usbgadget.Devices, error) {
func updateUsbRelatedConfig(wasAudioEnabled bool) error {
ensureConfigLoaded()
// Stop input audio before USB reconfiguration (input uses USB)
audioMutex.Lock()
stopInputLocked()
inRelay := inputRelay
inSource := inputSource
inputRelay = nil
inputSource = nil
audioMutex.Unlock()
if inRelay != nil {
inRelay.Stop()
}
if inSource != nil {
inSource.Disconnect()
}
if err := gadget.UpdateGadgetConfig(); err != nil {
return fmt.Errorf("failed to write gadget config: %w", err)
}

View File

@ -133,7 +133,7 @@ func Main() {
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
<-sigs
logger.Log().Msg("JetKVM Shutting Down")
logger.Info().Msg("JetKVM Shutting Down")
stopAudio()

View File

@ -12,19 +12,19 @@ import notifications from "../notifications";
export default function SettingsAudioRoute() {
const { send } = useJsonRpc();
const settings = useSettingsStore();
const { setAudioOutputEnabled, setAudioInputAutoEnable, audioOutputEnabled, audioInputAutoEnable } = useSettingsStore();
useEffect(() => {
send("getAudioOutputEnabled", {}, (resp: JsonRpcResponse) => {
if ("error" in resp) return;
settings.setAudioOutputEnabled(resp.result as boolean);
setAudioOutputEnabled(resp.result as boolean);
});
send("getAudioInputAutoEnable", {}, (resp: JsonRpcResponse) => {
if ("error" in resp) return;
settings.setAudioInputAutoEnable(resp.result as boolean);
setAudioInputAutoEnable(resp.result as boolean);
});
}, [send, settings]);
}, [send, setAudioOutputEnabled, setAudioInputAutoEnable]);
const handleAudioOutputEnabledChange = (enabled: boolean) => {
send("setAudioOutputEnabled", { enabled }, (resp: JsonRpcResponse) => {
@ -35,7 +35,7 @@ export default function SettingsAudioRoute() {
notifications.error(errorMsg);
return;
}
settings.setAudioOutputEnabled(enabled);
setAudioOutputEnabled(enabled);
const successMsg = enabled ? m.audio_output_enabled() : m.audio_output_disabled();
notifications.success(successMsg);
});
@ -47,7 +47,7 @@ export default function SettingsAudioRoute() {
notifications.error(String(resp.error.data || m.unknown_error()));
return;
}
settings.setAudioInputAutoEnable(enabled);
setAudioInputAutoEnable(enabled);
const successMsg = enabled
? m.audio_input_auto_enable_enabled()
: m.audio_input_auto_enable_disabled();
@ -67,7 +67,7 @@ export default function SettingsAudioRoute() {
description={m.audio_settings_output_description()}
>
<Checkbox
checked={settings.audioOutputEnabled || false}
checked={audioOutputEnabled || false}
onChange={(e) => handleAudioOutputEnabledChange(e.target.checked)}
/>
</SettingsItem>
@ -77,7 +77,7 @@ export default function SettingsAudioRoute() {
description={m.audio_settings_auto_enable_microphone_description()}
>
<Checkbox
checked={settings.audioInputAutoEnable || false}
checked={audioInputAutoEnable || false}
onChange={(e) => handleAudioInputAutoEnableChange(e.target.checked)}
/>
</SettingsItem>