Skip to content

Commit ef75217

Browse files
Address PR #476 review comments (10 fixes)
- ToolSettingsView (greptile P1): mark `isAtEnd` getter `mutating get` to fix Swift compile error from calling mutating `skipWhitespace()`. - RunAnywhere+TextGeneration (greptile P2): remove dead `Task.isCancelled` check in C-callback token path — always false (no ambient task); real cancellation already flows via `cancel_requested` atomic in C++. - tool_calling.cpp (greptile P2): flat-args fallback now emits numbers, booleans, and null verbatim via `is_json_scalar_literal()` instead of stringifying everything. `{"count":5}` stays `{"count":5}`. - RunAnywhere+TextGeneration (greptile P2): add `NOTE` doc comment to `ThinkingContentParser.extract` documenting single-`<think>`-block behavior. - CppBridge shutdown (greptile P2): replace Task.detached + semaphore.wait with DispatchQueue.global().async + Task + semaphore to avoid cooperative thread-pool starvation. Synchronous shutdown() signature preserved, race fix retained. - lifecycle_manager.cpp (coderabbit Critical): auto-unload now mirrors the existing unload fence — uses unique_lock, waits on service_cv for refcount==0, stores nullptr into atomic before destroy_fn, null-checks old_service. - RunAnywhere+TextGeneration (coderabbit Critical): remove the second error-handling path — C++ errorCallback already consumes the retained context via takeRetainedValue, so the duplicate release + finish + markFailed caused double-release/double-finish. - BenchmarkViewModel (coderabbit Major): stop coercing empty selectedModelIds to nil in runBenchmarks() — nil was interpreted as "no filter → run all", so "None" inverted behavior. - BenchmarkDashboardView (coderabbit Minor): fix copy inconsistency — header text now says "gradient images" to match per-row label. - build-ios.sh (coderabbit Major): replace fragile ar-extract-repack (loses members with shared basenames) with in-place `ar -d` to remove flag.cc.o/init.cc.o specifically.
1 parent 698fe6f commit ef75217

8 files changed

Lines changed: 85 additions & 38 deletions

File tree

examples/ios/RunAnywhereAI/RunAnywhereAI/Features/Benchmarks/ViewModels/BenchmarkViewModel.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,9 @@ final class BenchmarkViewModel {
111111
var run = BenchmarkRun(deviceInfo: deviceInfo)
112112

113113
do {
114-
let modelFilter = selectedModelIds.isEmpty ? nil : selectedModelIds
115114
let output = try await runner.runBenchmarks(
116115
categories: selectedCategories,
117-
modelIds: modelFilter
116+
modelIds: selectedModelIds
118117
) { [weak self] update in
119118
Task { @MainActor in
120119
self?.progress = update.progress

examples/ios/RunAnywhereAI/RunAnywhereAI/Features/Benchmarks/Views/BenchmarkDashboardView.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ struct BenchmarkDashboardView: View {
3131
// Benchmark Suite Info
3232
Section {
3333
VStack(alignment: .leading, spacing: AppSpacing.small) {
34-
Text("Each category runs deterministic scenarios against every downloaded model of that type. Synthetic inputs (silent audio, sine waves, solid-color images) ensure reproducible results.")
34+
Text("Each category runs deterministic scenarios against every downloaded model of that type. Synthetic inputs (silent audio, sine waves, gradient images) ensure reproducible results.")
3535
.font(AppTypography.caption)
3636
.foregroundColor(AppColors.textSecondary)
3737
}

examples/ios/RunAnywhereAI/RunAnywhereAI/Features/Settings/ToolSettingsView.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,10 @@ enum SafeMathEvaluator {
435435
}
436436

437437
var isAtEnd: Bool {
438-
skipWhitespace()
439-
return index >= scalars.count
438+
mutating get {
439+
skipWhitespace()
440+
return index >= scalars.count
441+
}
440442
}
441443

442444
mutating func skipWhitespace() {

sdk/runanywhere-commons/scripts/build-ios.sh

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -734,18 +734,10 @@ create_backend_xcframework() {
734734
continue
735735
fi
736736

737-
# For MetalRT: strip sentencepiece's flag.cc.o to avoid duplicate _FLAGS_help with sherpa-onnx
737+
# For MetalRT: strip sentencepiece's flag.cc.o/init.cc.o to avoid duplicate _FLAGS_help with sherpa-onnx
738738
if [[ "$BACKEND_NAME" == "metalrt" ]]; then
739-
local TMPSTRIP
740-
TMPSTRIP=$(mktemp -d) || { log_error "Failed to create temp directory"; exit 1; }
741-
cd "$TMPSTRIP"
742-
ar x "${FRAMEWORK_DIR}/${FRAMEWORK_NAME}"
743-
rm -f flag.cc.o init.cc.o
744-
rm -f "${FRAMEWORK_DIR}/${FRAMEWORK_NAME}"
745-
ar rcs "${FRAMEWORK_DIR}/${FRAMEWORK_NAME}" *.o
746-
cd -
747-
rm -rf "$TMPSTRIP"
748-
log_info " ${PLATFORM}: Stripped duplicate flag.cc.o from bundle"
739+
ar -d "${FRAMEWORK_DIR}/${FRAMEWORK_NAME}" flag.cc.o init.cc.o 2>/dev/null || true
740+
log_info " ${PLATFORM}: Stripped duplicate flag.cc.o/init.cc.o from bundle"
749741
fi
750742

751743
# For MetalRT: copy default.metallib into the framework as a resource

sdk/runanywhere-commons/src/core/capabilities/lifecycle_manager.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ rac_result_t rac_lifecycle_load(rac_handle_t handle, const char* model_path, con
169169
}
170170

171171
auto* mgr = static_cast<LifecycleManager*>(handle);
172-
std::lock_guard<std::mutex> lock(mgr->mutex);
172+
std::unique_lock<std::mutex> lock(mgr->mutex);
173173

174174
// Check if already loaded with same path - skip duplicate events
175175
// Mirrors Swift: if await lifecycle.currentResourceId == modelId
@@ -184,17 +184,23 @@ rac_result_t rac_lifecycle_load(rac_handle_t handle, const char* model_path, con
184184
// Auto-unload previous model if a different one is loaded.
185185
// Without this, the old service leaks resources (GPU memory, file handles)
186186
// and the new service creation may fail due to resource contention.
187-
if (mgr->state.load() == RAC_LIFECYCLE_STATE_LOADED && mgr->current_service != nullptr) {
187+
if (mgr->state.load() == RAC_LIFECYCLE_STATE_LOADED && mgr->current_service.load() != nullptr) {
188+
// Wait for all acquired services to be released (mirror unload fence)
189+
mgr->service_cv.wait(lock, [mgr] { return mgr->service_refcount.load() == 0; });
190+
188191
RAC_LOG_INFO(mgr->logger_category.c_str(),
189192
"Auto-unloading previous model '%s' before loading '%s'",
190193
mgr->current_model_id.c_str(), model_id);
191194

192-
if (mgr->destroy_fn != nullptr) {
193-
mgr->destroy_fn(mgr->current_service, mgr->user_data);
195+
// Store nullptr BEFORE calling destroy_fn so that concurrent cancel()
196+
// reads via get_service() see nullptr rather than a dangling pointer.
197+
rac_handle_t old_service = mgr->current_service.load();
198+
mgr->current_service.store(nullptr);
199+
if (mgr->destroy_fn != nullptr && old_service != nullptr) {
200+
mgr->destroy_fn(old_service, mgr->user_data);
194201
}
195202
track_lifecycle_event(mgr, "unloaded", mgr->current_model_id.c_str(), 0.0, RAC_SUCCESS);
196203

197-
mgr->current_service = nullptr;
198204
mgr->current_model_path.clear();
199205
mgr->current_model_id.clear();
200206
mgr->current_model_name.clear();

sdk/runanywhere-commons/src/features/llm/tool_calling.cpp

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,42 @@ static std::string escape_json_string(const char* str) {
564564
return result;
565565
}
566566

567+
/**
568+
* @brief Classify a raw scalar token as a JSON literal (number, boolean, or null).
569+
*
570+
* Used to decide whether an extracted value should be emitted verbatim into
571+
* reconstructed JSON (true) or wrapped in quotes as a string (false). Accepts
572+
* standard JSON number syntax plus the literals `true`, `false`, `null`.
573+
*/
574+
static bool is_json_scalar_literal(const char* s) {
575+
if (!s || !*s) {
576+
return false;
577+
}
578+
579+
// Boolean and null literals
580+
if (strcmp(s, "true") == 0 || strcmp(s, "false") == 0 || strcmp(s, "null") == 0) {
581+
return true;
582+
}
583+
584+
// JSON number: optional '-', digits, optional fraction, optional exponent
585+
size_t i = 0;
586+
if (s[i] == '-') i++;
587+
if (!isdigit(static_cast<unsigned char>(s[i]))) return false;
588+
while (isdigit(static_cast<unsigned char>(s[i]))) i++;
589+
if (s[i] == '.') {
590+
i++;
591+
if (!isdigit(static_cast<unsigned char>(s[i]))) return false;
592+
while (isdigit(static_cast<unsigned char>(s[i]))) i++;
593+
}
594+
if (s[i] == 'e' || s[i] == 'E') {
595+
i++;
596+
if (s[i] == '+' || s[i] == '-') i++;
597+
if (!isdigit(static_cast<unsigned char>(s[i]))) return false;
598+
while (isdigit(static_cast<unsigned char>(s[i]))) i++;
599+
}
600+
return s[i] == '\0';
601+
}
602+
567603
// =============================================================================
568604
// JSON NORMALIZATION
569605
// =============================================================================
@@ -719,8 +755,16 @@ static bool extract_tool_name_and_args(const char* json_obj, char** out_tool_nam
719755
if (kval_is_obj) {
720756
flat_args += "\"" + escaped_key + "\":" + std::string(kval);
721757
} else if (kval) {
722-
std::string escaped_val = escape_json_string(kval);
723-
flat_args += "\"" + escaped_key + "\":\"" + escaped_val + "\"";
758+
// Emit JSON numbers/booleans/null verbatim; quote strings.
759+
// extract_json_value strips quotes from string values but
760+
// preserves raw scalars, so we classify the stripped value
761+
// using JSON scalar syntax rules.
762+
if (is_json_scalar_literal(kval)) {
763+
flat_args += "\"" + escaped_key + "\":" + std::string(kval);
764+
} else {
765+
std::string escaped_val = escape_json_string(kval);
766+
flat_args += "\"" + escaped_key + "\":\"" + escaped_val + "\"";
767+
}
724768
}
725769
free(kval);
726770
first = false;

sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/CppBridge.swift

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -185,19 +185,26 @@ public enum CppBridge {
185185
// causing use-after-free inside the C++ layer and allowing reinit
186186
// while destruction is still in flight.
187187
//
188-
// The destroy() methods are actor-isolated but not @MainActor, so
189-
// blocking a background/non-MainActor caller on a semaphore is safe.
188+
// We hop to a GCD global queue (not the Swift cooperative thread pool)
189+
// and block that GCD worker on a semaphore. Blocking a cooperative
190+
// thread-pool thread via `Task.detached { ... }` + `semaphore.wait()`
191+
// on the caller risks priority inversion / pool starvation, since the
192+
// detached task and the caller may share the same limited pool. The
193+
// GCD global pool is separate and safe to block briefly.
194+
//
190195
// reset() is not annotated @MainActor, so callers are expected to
191196
// invoke it off the main thread.
192197
let semaphore = DispatchSemaphore(value: 0)
193-
Task.detached {
194-
await LLM.shared.destroy()
195-
await STT.shared.destroy()
196-
await TTS.shared.destroy()
197-
await VAD.shared.destroy()
198-
await VoiceAgent.shared.destroy()
199-
await VLM.shared.destroy()
200-
semaphore.signal()
198+
DispatchQueue.global(qos: .userInitiated).async {
199+
Task {
200+
await LLM.shared.destroy()
201+
await STT.shared.destroy()
202+
await TTS.shared.destroy()
203+
await VAD.shared.destroy()
204+
await VoiceAgent.shared.destroy()
205+
await VLM.shared.destroy()
206+
semaphore.signal()
207+
}
201208
}
202209
semaphore.wait()
203210

sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/LLM/RunAnywhere+TextGeneration.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,7 @@ public extension RunAnywhere {
231231
}
232232

233233
if streamResult != RAC_SUCCESS {
234-
Unmanaged<LLMStreamCallbackContext>.fromOpaque(contextPtr).release()
235-
let error = SDKError.llm(.generationFailed, "Stream generation failed: \(streamResult)")
236-
continuation.finish(throwing: error)
237-
await collector.markFailed(error)
234+
return
238235
}
239236
}
240237
}
@@ -258,7 +255,6 @@ private enum LLMStreamCallbacks {
258255
static func create() -> Callbacks {
259256
let tokenCallback: TokenFn = { tokenPtr, userData -> rac_bool_t in
260257
guard let tokenPtr = tokenPtr, let userData = userData else { return RAC_TRUE }
261-
if Task.isCancelled { return RAC_FALSE }
262258
let ctx = Unmanaged<LLMStreamCallbackContext>.fromOpaque(userData).takeUnretainedValue()
263259
let token = String(cString: tokenPtr)
264260
Task {
@@ -316,6 +312,7 @@ private final class LLMStreamCallbackContext: @unchecked Sendable {
316312

317313
enum ThinkingContentParser {
318314
/// Extracts `<think>...</think>` content from generated text.
315+
/// - NOTE: Only the first `<think>` block is extracted; additional blocks are left inline in the response text.
319316
/// - Returns: Tuple of (responseText, thinkingContent). If no tags found, responseText = original text, thinkingContent = nil.
320317
static func extract(from text: String) -> (text: String, thinking: String?) {
321318
guard let startRange = text.range(of: "<think>"),

0 commit comments

Comments
 (0)