Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion FirebaseAI/Sources/Chat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public final class Chat: Sendable {
let request = history + newContent
let stream = try model.generateContentStream(request, generationConfig: generationConfig)
return AsyncThrowingStream { continuation in
Task {
let task = Task {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The task should check for cancellation before modifying the chat history. If the stream is cancelled, appending newContent and the partial aggregatedContent to _history (lines 138-142) might lead to an inconsistent chat state. Consider adding a check like guard !Task.isCancelled else { return } before updating the history.

var aggregatedContent: [ModelContent] = []

do {
Expand All @@ -142,6 +142,9 @@ public final class Chat: Sendable {
self._history.append(aggregated)
continuation.finish()
}
continuation.onTermination = { _ in
task.cancel()
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion FirebaseAI/Sources/GenerativeAIService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ struct GenerativeAIService {
func loadRequestStream<T: GenerativeAIRequest>(request: T)
-> AsyncThrowingStream<T.Response, Error> where T: Sendable {
return AsyncThrowingStream { continuation in
Task {
let task = Task {
let urlRequest: URLRequest
do {
urlRequest = try await self.urlRequest(request: request)
Expand Down Expand Up @@ -160,6 +160,9 @@ struct GenerativeAIService {

continuation.finish(throwing: nil)
}
continuation.onTermination = { _ in
task.cancel()
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion FirebaseAI/Sources/GenerativeModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public final class GenerativeModel: Sendable {

return AsyncThrowingStream { continuation in
let responseStream = generativeAIService.loadRequestStream(request: generateContentRequest)
Task {
let task = Task {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Consider checking for task cancellation before finishing the continuation. If the task is cancelled during the loop, the code currently proceeds to line 357 and might call continuation.finish(throwing:) with an emptyContent error if no responses were yielded yet. It would be cleaner to exit early if Task.isCancelled is true.

do {
var didYieldResponse = false
for try await response in responseStream {
Expand Down Expand Up @@ -368,6 +368,9 @@ public final class GenerativeModel: Sendable {
return
}
}
continuation.onTermination = { _ in
task.cancel()
}
}
}

Expand Down
Loading