Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenAI Chat LLM thread safety #281

Closed
danielchalef opened this issue Sep 5, 2023 · 2 comments · May be fixed by #1080
Closed

OpenAI Chat LLM thread safety #281

danielchalef opened this issue Sep 5, 2023 · 2 comments · May be fixed by #1080

Comments

@danielchalef
Copy link

danielchalef commented Sep 5, 2023 •
edited
Loading

I'm seeing data race warnings when using an openai.Chat instance across multiple goroutines.

It appears Client.createChat is writing to c.baseURL when called:

langchaingo/llms/openai/internal/openaiclient/chat.go

Line 147 in c85d396

c.baseURL = defaultBaseURL

Is the lack of thread safety expected for LLM clients?

WARNING: DATA RACE
Read at 0x00c0003022a0 by goroutine 207:
github.com/tmc/langchaingo/llms/openai/internal/openaiclient.(*Client).createChat()
  /Users/danielchalef/go/pkg/mod/github.com/tmc/[email protected]/llms/openai/internal/openaiclient/chat.go:145 +0x128
github.com/tmc/langchaingo/llms/openai/internal/openaiclient.(*Client).CreateChat()
  /Users/danielchalef/go/pkg/mod/github.com/tmc/[email protected]/llms/openai/internal/openaiclient/openaiclient.go:137 +0x174
github.com/tmc/langchaingo/llms/openai.(*Chat).Generate()
  /Users/danielchalef/go/pkg/mod/github.com/tmc/[email protected]/llms/openai/openaillm_chat.go:113 +0x5a0
github.com/tmc/langchaingo/llms/openai.(*Chat).Call()
  /Users/danielchalef/go/pkg/mod/github.com/tmc/[email protected]/llms/openai/openaillm_chat.go:42 +0x8c
github.com/getzep/zep/pkg/llms.(*ZepOpenAILLM).Call()
<snip>
Goroutine 207 (running) created at:
github.com/getzep/zep/pkg/extractors.(*IntentExtractor).Extract()
  /Users/danielchalef/dev/zep/pkg/extractors/intent_extractor.go:41 +0x1b8
github.com/getzep/zep/pkg/extractors.(*IntentExtractor).Notify()
  /Users/danielchalef/dev/zep/pkg/extractors/intent_extractor.go:154 +0x50
github.com/getzep/zep/pkg/store.(*BaseMemoryStore[go.shape.*uint8]).NotifyExtractors.func1()
  /Users/danielchalef/dev/zep/pkg/store/memory_base.go:32 +0x6c
github.com/getzep/zep/pkg/store.(*BaseMemoryStore[go.shape.*uint8]).NotifyExtractors.func2()
  /Users/danielchalef/dev/zep/pkg/store/memory_base.go:36 +0x54
Goroutine 200 (running) created at:
github.com/getzep/zep/pkg/extractors.(*IntentExtractor).Extract()
  /Users/danielchalef/dev/zep/pkg/extractors/intent_extractor.go:41 +0x1b8
github.com/getzep/zep/pkg/extractors.(*IntentExtractor).Notify()
  /Users/danielchalef/dev/zep/pkg/extractors/intent_extractor.go:154 +0x50
github.com/getzep/zep/pkg/store.(*BaseMemoryStore[go.shape.*uint8]).NotifyExtractors.func1()
  /Users/danielchalef/dev/zep/pkg/store/memory_base.go:32 +0x6c
github.com/getzep/zep/pkg/store.(*BaseMemoryStore[go.shape.*uint8]).NotifyExtractors.func2()
  /Users/danielchalef/dev/zep/pkg/store/memory_base.go:36 +0x54
@tmc
Copy link
Owner

tmc commented Mar 19, 2024

This code has been refactored since then and I believe this race was factored away. Very much want these types to pass any race detection checks (would love a test that adds/checks that!).

@tmc tmc closed this as completed Mar 19, 2024
@nktks
Copy link

nktks commented Dec 2, 2024

Hello.
It still happens at latest commit 238d1c7

I tested at https://gist.github.com/nktks/2d96fa72bf3d094c85bb057d63d673a7

@nktks nktks mentioned this issue Dec 2, 2024
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: fix data races nktks/langchaingo
3 participants
@tmc @danielchalef @nktks