fix(whatsmeow): create the sqlstore container once instead of on every StartClient - #194
Open
EcoosUP wants to merge 1 commit into
Open
fix(whatsmeow): create the sqlstore container once instead of on every StartClient#194EcoosUP wants to merge 1 commit into
EcoosUP wants to merge 1 commit into
Conversation
…y StartClient
StartClient called sqlstore.New on every invocation and never closed the
previous container. Each sqlstore.New opens its own *sql.DB, and a *sql.DB that
is never closed keeps its TCP connections open forever.
This is harmless while instances stay up. It becomes fatal when one instance
loops: a device logged out from the phone reconnects, finds no session, emits a
QR, nobody scans it, the max QR count forces a logout, that emits Disconnected,
and the cycle restarts. Every turn leaked a whole connection pool.
Measured on a 500-connection Postgres: a single logged-out instance produced 110
reconnects in 50 minutes and exhausted the server. After that, every OTHER
instance that dropped its websocket failed to return with
Failed to create container: pq: sorry, too many clients already
One dead instance took every other number down with it.
The DSN is identical for all instances, so a single container serves them all.
After the change: 10 StartClient calls, 4 connections in use.
The error is deliberately not memoized (Mutex, not sync.Once) so a database that
is briefly unreachable at startup does not poison the whole process.
Reviewer's GuideFixes connection-pool exhaustion by creating the whatsmeow SQL store container once and sharing it across StartClient calls, while allowing transient initialization failures to be retried. Sequence diagram for shared whatsmeow container initializationsequenceDiagram
participant StartClient
participant sharedContainer
participant sqlstore
participant Database
StartClient->>sharedContainer: sharedContainer(w)
sharedContainer->>sharedContainer: Lock sharedContainerMu
alt sharedStore != nil
sharedContainer-->>StartClient: sharedStore
else sharedStore is nil
sharedContainer->>sqlstore: New(context.Background(), driver, dsn, dbLog)
alt initialization succeeds
sqlstore->>Database: Open connection pool
Database-->>sqlstore: Pool ready
sharedContainer->>sharedContainer: sharedStore = container
sharedContainer-->>StartClient: container
else initialization fails
sqlstore-->>sharedContainer: error
sharedContainer-->>StartClient: error
StartClient->>StartClient: Retry on a later call
end
end
sharedContainer->>sharedContainer: Unlock sharedContainerMu
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. The first StartClient call fixes the process-wide database container, so if service instances use different SQLite paths or database credentials, later clients could read or write session data in the wrong database. Reverting stops further misrouting, but records written through the wrong container would remain and require cleanup or recovery.
felipersd8
added a commit
to felipersd8/evolution-go
that referenced
this pull request
Sep 14, 2026
…y StartClient StartClient opened a new sqlstore container — its own *sql.DB pool — on every call and never closed the previous one. An instance that is not paired restarts its QR loop every couple of minutes, and every restart leaked a pool. In hours the Postgres hit max_connections and every instance start failed with "pq: sorry, too many clients already", so no QR code could be generated until the container was restarted. The DSN is the same for every instance, so one shared container serves them all. Patch from upstream PR evolution-foundation#194 (issues evolution-foundation#106, evolution-foundation#109, evolution-foundation#175, evolution-foundation#186), applied on top of 0.7.2 while upstream has not released a fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
StartClientcallssqlstore.Newon every invocation and never closes the previous container. There is nocontainer.Close()anywhere in the file.Each
sqlstore.Newopens its own*sql.DB. A*sql.DBthat is never closed keeps its TCP connections open for the lifetime of the process — the garbage collector does not reclaim them.This is harmless while instances stay connected. It becomes fatal when one instance loops, and a device that was logged out from the phone does exactly that:
Every turn of that loop leaks a whole connection pool.
Measured in production
A 500-connection Postgres, nine instances, one of them logged out from the phone at 21:06:
At 21:50 the pool was gone, and from that point on every other instance that dropped its websocket failed to come back:
Three unrelated numbers went silent for 14 hours because of one dead instance. All 500 connections were sitting
idlewhen we found them — nothing was using them.The fix
The DSN is identical for every instance (
w.config.PostgresAuthDB, or the same sqlite path), so there was never a reason for a container per instance. This creates it once and shares it.After the change, same deployment: 10
StartClientcalls, 4 connections in use.Two notes on the implementation
The error is not memoized. A
sync.Oncewould let a database that is briefly unreachable at the first attempt poison the process for its entire lifetime. AMutexguarding a nil check retries until it succeeds, and only the success is stored.var err errorwas removed from the top ofStartClientbecause the new:=declares it. No behaviour change.Testing
Built against
main(go build ./..., clean) and running in production on two separate Evolution GO processes.Happy to split this differently or adjust naming if you prefer.
Summary by Sourcery
Reuse a single whatsmeow SQL store container across client starts to prevent connection exhaustion during reconnect loops.
Bug Fixes:
Enhancements: