Skip to content

Commit 1792730

Browse files
fix: address PR #471 review comments (10/11)
Swift SDK: - RunAnywhere.swift: atomically check-or-create _servicesInitTask under a DispatchQueue so concurrent completeServicesInitialization() callers can't spawn duplicate init tasks. Also clear the task on success (not just failure) so the completed Task is released. (coderabbit #3082258780, greptile #3082192287) - CppBridge+VAD.swift: clear loadedModelId before calling rac_vad_component_load_model — the C side unloads first, so on load failure our Swift mirror must already be nil for a retry to work. (coderabbit #3082258776) - WhisperKitSTTService.swift: replace legacy vDSP_measqv+sqrt with the Swift-overlay vDSP.rootMeanSquare(_:). (coderabbit #3082258785) Commons (C++): - onnx_backend.cpp (VAD model resolution): collect all *.onnx files, sort lexicographically, pick the first — previously picked whatever readdir() happened to return first, which is filesystem-dependent. (coderabbit #3082258759) - vad_component.cpp: if model_service->ops->start() returns non-success, fully roll back (destroy impl, free service, clear is_model_loaded/loaded_model_id) and propagate the error instead of lying about a loaded-but-not-running service. (coderabbit #3082258769) Playground (YapRun + keyboard extension): - PlaygroundViewModel.swift: gate FlowSessionManager.shared.isActive check behind #if os(iOS) — FlowSessionManager is iOS-only and the unguarded reference broke the macOS target. (coderabbit #3082258684) - FlowSessionManager.swift: guard deliverResult() with `case .transcribing = sessionPhase` so a Task.detached that completes after endSession()/killSession() doesn't overwrite SharedDataBridge with a discarded result. (coderabbit #3082258728) - KeyboardViewController.swift: remove the unconditional UIPasteboard.general.string = text copy — duplicating every dictated utterance to the global pasteboard is a privacy leak. (coderabbit #3082258734) - SharedDataBridge.swift: (1) audioLevel setter now posts audioLevelChanged so peer-process caches refresh; (2) drop the _cachedHeartbeat cache entirely and read/write UserDefaults directly — heartbeat ticks every second without a state transition so the cache was stale by definition. (coderabbit #3082258739) Not addressed: - rac_vad_component_is_loaded returning rac_bool_t (coderabbit #3082258749) — intentionally left as rac_bool_t to match the established project convention for is_X query APIs (rac_stt_ component_is_loaded, rac_server_is_running, rac_framework_is_ platform_service, ~20 more). Changing only the VAD one would break consistency without a concrete call-site benefit. Verification: - sdk/runanywhere-commons: 51/51 existing tests pass (test_core, test_extraction, test_download_orchestrator). - sdk/runanywhere-commons ONNX backend: builds clean. - sdk/runanywhere-swift: `swift build` complete (425 modules). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 525243e commit 1792730

9 files changed

Lines changed: 82 additions & 39 deletions

File tree

Playground/YapRun/YapRun/Features/Playground/PlaygroundViewModel.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,14 @@ final class PlaygroundViewModel {
6868
return
6969
}
7070

71-
// Prevent conflict with active voice keyboard session
71+
// Prevent conflict with active voice keyboard session (iOS-only).
72+
// FlowSessionManager is compiled `#if os(iOS)` so this check is skipped on macOS.
73+
#if os(iOS)
7274
guard !FlowSessionManager.shared.isActive else {
7375
errorMessage = "Voice keyboard session is active. End it first."
7476
return
7577
}
78+
#endif
7679

7780
let permitted = await audioCapture.requestPermission()
7881
guard permitted else {

Playground/YapRun/YapRun/Features/VoiceKeyboard/FlowSessionManager.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,16 @@ final class FlowSessionManager: ObservableObject {
257257
try await RunAnywhere.transcribe(audio)
258258
}.value
259259
logger.info("Transcription complete: \"\(text)\"")
260+
261+
// Task.detached drops structured cancellation, so endSession()/killSession()
262+
// cannot cancel the transcription in flight. If the session was torn down
263+
// while transcription was running, discard the result rather than writing
264+
// it to SharedDataBridge after the session is idle.
265+
guard case .transcribing = sessionPhase else {
266+
logger.info("Session no longer transcribing — discarding result")
267+
return
268+
}
269+
260270
wordCount += text.split(separator: " ").count
261271

262272
if #available(iOS 16.1, *) {

Playground/YapRun/YapRunKeyboard/KeyboardViewController.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,10 @@ final class KeyboardViewController: UIInputViewController {
151151
SharedDataBridge.shared.lastInsertedText = text
152152
SharedDataBridge.shared.transcribedText = nil
153153

154-
// Copy to clipboard if we have full access
155-
if hasFullAccess {
156-
UIPasteboard.general.string = text
157-
}
154+
// Intentionally NOT copying to UIPasteboard: duplicating every dictated
155+
// utterance to the global pasteboard is a privacy leak (other apps with
156+
// pasteboard access would see it). A future opt-in setting or an
157+
// explicit "Copy" action can gate a pasteboard write if users want one.
158158
}
159159

160160
// MARK: - URL Opening

Playground/YapRun/YapRunKeyboard/SharedDataBridge.swift

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,11 @@ final class SharedDataBridge {
7373
// Caches are invalidated via Darwin notifications from the main app.
7474
private var _cachedSessionState: String
7575
private var _cachedAudioLevel: Float
76-
private var _cachedHeartbeat: Double
7776

7877
private init() {
7978
defaults = UserDefaults(suiteName: SharedConstants.appGroupID)
8079
_cachedSessionState = defaults?.string(forKey: SharedConstants.Keys.sessionState) ?? "idle"
8180
_cachedAudioLevel = defaults?.float(forKey: SharedConstants.Keys.audioLevel) ?? 0
82-
_cachedHeartbeat = defaults?.double(forKey: SharedConstants.Keys.lastHeartbeat) ?? 0
8381
registerCacheObservers()
8482
}
8583

@@ -102,13 +100,11 @@ final class SharedDataBridge {
102100
func refreshAllFromDefaults() {
103101
_cachedSessionState = defaults?.string(forKey: SharedConstants.Keys.sessionState) ?? "idle"
104102
_cachedAudioLevel = defaults?.float(forKey: SharedConstants.Keys.audioLevel) ?? 0
105-
_cachedHeartbeat = defaults?.double(forKey: SharedConstants.Keys.lastHeartbeat) ?? 0
106103
}
107104

108105
/// Refresh only session state from UserDefaults.
109106
private func refreshStateFromDefaults() {
110107
_cachedSessionState = defaults?.string(forKey: SharedConstants.Keys.sessionState) ?? "idle"
111-
_cachedHeartbeat = defaults?.double(forKey: SharedConstants.Keys.lastHeartbeat) ?? 0
112108
}
113109

114110
/// Refresh only audio level from UserDefaults.
@@ -165,17 +161,23 @@ final class SharedDataBridge {
165161
set {
166162
_cachedAudioLevel = newValue
167163
defaults?.set(newValue, forKey: SharedConstants.Keys.audioLevel)
164+
// Post so the peer process (keyboard extension ↔ main app) refreshes
165+
// its own _cachedAudioLevel — otherwise readers see stale values.
166+
DarwinNotificationCenter.shared.post(
167+
name: SharedConstants.DarwinNotifications.audioLevelChanged
168+
)
168169
}
169170
}
170171

171172
// MARK: - Heartbeat
172173

174+
/// Heartbeat is written ~every second by FlowSessionManager without a state
175+
/// transition, so caching + Darwin-notification invalidation would require
176+
/// a notification per second. Instead, read through to UserDefaults every
177+
/// time so remote readers always see a fresh value.
173178
var lastHeartbeatTimestamp: Double {
174-
get { _cachedHeartbeat }
175-
set {
176-
_cachedHeartbeat = newValue
177-
defaults?.set(newValue, forKey: SharedConstants.Keys.lastHeartbeat)
178-
}
179+
get { defaults?.double(forKey: SharedConstants.Keys.lastHeartbeat) ?? 0 }
180+
set { defaults?.set(newValue, forKey: SharedConstants.Keys.lastHeartbeat) }
179181
}
180182

181183
// MARK: - Last Inserted Text
@@ -220,7 +222,6 @@ final class SharedDataBridge {
220222
defaults?.set(Float(0), forKey: SharedConstants.Keys.audioLevel)
221223
defaults?.set(Double(0), forKey: SharedConstants.Keys.lastHeartbeat)
222224
_cachedAudioLevel = 0
223-
_cachedHeartbeat = 0
224225
sessionState = "idle"
225226
}
226227
}

sdk/runanywhere-commons/src/backends/onnx/onnx_backend.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
#include <dirent.h>
1818
#include <sys/stat.h>
1919

20+
#include <algorithm>
2021
#include <cctype>
2122
#include <cstdio>
2223
#include <cstring>
24+
#include <vector>
2325

2426
#include "rac/core/rac_logger.h"
2527

@@ -1153,23 +1155,28 @@ bool ONNXVAD::load_model(const std::string& model_path, VADModelType model_type,
11531155
model_path_ = model_path;
11541156
struct stat path_stat;
11551157
if (stat(model_path.c_str(), &path_stat) == 0 && S_ISDIR(path_stat.st_mode)) {
1156-
std::string resolved;
1158+
// Collect every `.onnx` filename and sort lexicographically so the
1159+
// choice is deterministic across runs (readdir() order is
1160+
// filesystem-dependent and not stable).
1161+
std::vector<std::string> candidates;
11571162
DIR* dir = opendir(model_path.c_str());
11581163
if (dir) {
11591164
struct dirent* entry;
11601165
while ((entry = readdir(dir)) != nullptr) {
11611166
std::string filename = entry->d_name;
11621167
if (filename.size() > 5 &&
11631168
filename.substr(filename.size() - 5) == ".onnx") {
1164-
resolved = model_path + "/" + filename;
1165-
RAC_LOG_DEBUG("ONNX.VAD", "Found VAD model file: %s", resolved.c_str());
1166-
break;
1169+
candidates.push_back(std::move(filename));
11671170
}
11681171
}
11691172
closedir(dir);
11701173
}
1171-
if (!resolved.empty()) {
1172-
model_path_ = resolved;
1174+
if (!candidates.empty()) {
1175+
std::sort(candidates.begin(), candidates.end());
1176+
model_path_ = model_path + "/" + candidates.front();
1177+
RAC_LOG_DEBUG("ONNX.VAD", "Found VAD model file: %s (%zu candidate%s)",
1178+
model_path_.c_str(), candidates.size(),
1179+
candidates.size() == 1 ? "" : "s");
11731180
} else {
11741181
RAC_LOG_ERROR("ONNX.VAD", "No .onnx file found in directory: %s", model_path.c_str());
11751182
return false;

sdk/runanywhere-commons/src/features/vad/vad_component.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,11 +447,22 @@ extern "C" rac_result_t rac_vad_component_load_model(rac_handle_t handle,
447447
component->is_model_loaded = true;
448448
component->loaded_model_id = model_id ? strdup(model_id) : nullptr;
449449

450-
// Start the model-based VAD
450+
// Start the model-based VAD. If start fails, roll back so `is_model_loaded`
451+
// does not lie about a non-running service.
451452
if (component->model_service->ops && component->model_service->ops->start) {
452453
result = component->model_service->ops->start(component->model_service->impl);
453454
if (result != RAC_SUCCESS) {
454-
RAC_LOG_WARNING("VAD.Component", "Model VAD start returned non-success, continuing");
455+
log_error("VAD.Component", "Model VAD start failed: %d — rolling back load", result);
456+
if (component->model_service->ops->destroy) {
457+
component->model_service->ops->destroy(component->model_service->impl);
458+
}
459+
free(const_cast<char*>(component->model_service->model_id));
460+
free(component->model_service);
461+
component->model_service = nullptr;
462+
component->is_model_loaded = false;
463+
free(component->loaded_model_id);
464+
component->loaded_model_id = nullptr;
465+
return result;
455466
}
456467
}
457468

sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+VAD.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ extension CppBridge {
7777

7878
let handle = try getHandle()
7979

80+
// `rac_vad_component_load_model` unloads any previously loaded model
81+
// first. If the subsequent load fails, the C++ side is already
82+
// unloaded, so clear our mirror before the call so a retry isn't
83+
// skipped by the `loadedModelId != modelId` fast path above.
84+
loadedModelId = nil
85+
8086
let result = modelPath.withCString { pathPtr in
8187
modelId.withCString { idPtr in
8288
modelName.withCString { namePtr in

sdk/runanywhere-swift/Sources/RunAnywhere/Public/RunAnywhere.swift

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ public enum RunAnywhere {
7171
/// Concurrent callers of completeServicesInitialization() await this shared task
7272
/// instead of racing through the init logic with an unprotected boolean guard.
7373
private static var _servicesInitTask: Task<Void, Error>?
74+
/// Serializes the check-and-set on `_servicesInitTask` so two concurrent callers
75+
/// can't both observe `nil` and spawn duplicate init tasks.
76+
private static let _servicesInitLock = DispatchQueue(label: "com.runanywhere.sdk.servicesInit")
7477

7578
// MARK: - SDK State
7679

@@ -316,24 +319,28 @@ public enum RunAnywhere {
316319
return
317320
}
318321

319-
// If another task is already running Phase 2, join it instead of
320-
// starting a duplicate. This prevents the race between the background
321-
// Task.detached spawned by initialize() and any caller that reaches
322-
// completeServicesInitialization() via ensureServicesReady().
323-
if let existingTask = _servicesInitTask {
324-
try await existingTask.value
325-
return
326-
}
327-
328-
let task = Task<Void, Error> {
329-
try await _performServicesInitialization()
322+
// Atomically check-or-create the shared init task. Without the lock, two
323+
// concurrent callers could both see `_servicesInitTask == nil` and spawn
324+
// duplicate init tasks (e.g., the background Task.detached from initialize()
325+
// racing a caller via ensureServicesReady()).
326+
let task: Task<Void, Error> = _servicesInitLock.sync {
327+
if let existingTask = _servicesInitTask {
328+
return existingTask
329+
}
330+
let newTask = Task<Void, Error> {
331+
try await _performServicesInitialization()
332+
}
333+
_servicesInitTask = newTask
334+
return newTask
330335
}
331-
_servicesInitTask = task
332336

333337
do {
334338
try await task.value
339+
// Clear on success so the completed Task is released; subsequent calls
340+
// hit the fast path via `hasCompletedServicesInit`.
341+
_servicesInitLock.sync { _servicesInitTask = nil }
335342
} catch {
336-
_servicesInitTask = nil
343+
_servicesInitLock.sync { _servicesInitTask = nil }
337344
throw error
338345
}
339346
}

sdk/runanywhere-swift/Sources/WhisperKitRuntime/WhisperKitSTTService.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,7 @@ public actor WhisperKitSTTService {
191191
/// Root-mean-square energy using Accelerate (O(n) vectorized).
192192
private func rmsEnergy(_ samples: [Float]) -> Float {
193193
guard !samples.isEmpty else { return 0 }
194-
var meanSquare: Float = 0
195-
vDSP_measqv(samples, 1, &meanSquare, vDSP_Length(samples.count))
196-
return sqrt(meanSquare)
194+
return vDSP.rootMeanSquare(samples)
197195
}
198196

199197
/// In-place gain using Accelerate (O(n) vectorized).

0 commit comments

Comments
 (0)