Skip to content

Fix bad_weak_ptr when close a ClientConnection during construction - #350

Merged
BewareMyPower merged 1 commit into
apache:mainfrom
BewareMyPower:bewaremypower/fix-cnx-bad-weak-ptr
Nov 17, 2023
Merged

Fix bad_weak_ptr when close a ClientConnection during construction#350
BewareMyPower merged 1 commit into
apache:mainfrom
BewareMyPower:bewaremypower/fix-cnx-bad-weak-ptr

Conversation

@BewareMyPower

@BewareMyPowerBewareMyPower commented Nov 16, 2023

Copy link
Copy Markdown
Contributor

Fixes#348

Fixes#349

Motivation

When close is called in ClientConnection's constructor,
shared_from_this() will be called, which results in a
std::bad_weak_ptr error. This error does not happen before
#317 because
shared_from_this() could only be called when the producers or
consumers field is not empty.

Modifications

Add a ResultException and throw it if there is a failure in
ClientConnection's constructor and catch it in
ConnectionPool::getConnectionAsync. Then retrieve the result and
return the failed future.

Since close is now always called on a constructed ClientConnection
object, remove the 2nd parameter.

In addition, check authentication_ even for non-TLS URLs. Otherwise,
the null authentication will be used to construct CommandConnect.

Add testInvalidPlugin and testTlsConfigError to verify the changes.

@BewareMyPowerBewareMyPower added the bug Something isn't working label Nov 16, 2023
@BewareMyPowerBewareMyPower self-assigned this Nov 16, 2023
@BewareMyPowerBewareMyPower added this to the 3.4.1 milestone Nov 16, 2023
Comment threadlib/ClientConnection.h Outdated
@BewareMyPower
BewareMyPowerforce-pushed the bewaremypower/fix-cnx-bad-weak-ptr branch 2 times, most recently from 01ea0b1 to ba714eaCompareNovember 17, 2023 08:08
@BewareMyPower

Copy link
Copy Markdown
ContributorAuthor

@RobertIndie I used an exception-based way to fix this issue now so close is always called on a ClientConnection object that completes the construction successfully.

@BewareMyPower
BewareMyPower marked this pull request as draft November 17, 2023 08:11
@BewareMyPower

Copy link
Copy Markdown
ContributorAuthor

The Oauth2Test failed, I will check the failed test first.

Fixesapache#348Fixesapache#349
### Motivation
When `close` is called in `ClientConnection`'s constructor,
`shared_from_this()` will be called, which results in a
`std::bad_weak_ptr` error. This error does not happen before
apache#317 because
`shared_from_this()` could only be called when the `producers` or
`consumers` field is not empty.
### Modifications
Throw `ResultAuthenticationError` when there is anything wrong with
authentication in `ClientConnection`'s constructor. Then catch the
result in `ConnectionPool::getConnectionAsync` and returned the failed
future.
In addition, check `authentication_` even for non-TLS URLs. Otherwise,
the null authentication will be used to construct `CommandConnect`.
Add `testInvalidPlugin` and `testTlsConfigError` to verify the changes.
@BewareMyPower
BewareMyPowerforce-pushed the bewaremypower/fix-cnx-bad-weak-ptr branch from 688905d to 2a4e5b3CompareNovember 17, 2023 08:48
@BewareMyPower
BewareMyPower marked this pull request as ready for review November 17, 2023 08:49
@BewareMyPower
BewareMyPower merged commit 7bb94f4 into apache:mainNov 17, 2023
@BewareMyPower
BewareMyPower deleted the bewaremypower/fix-cnx-bad-weak-ptr branch November 17, 2023 09:51
BewareMyPower added a commit that referenced this pull request Nov 17, 2023
)
Fixes#348Fixes#349
### Motivation
When `close` is called in `ClientConnection`'s constructor,
`shared_from_this()` will be called, which results in a
`std::bad_weak_ptr` error. This error does not happen before
#317 because
`shared_from_this()` could only be called when the `producers` or
`consumers` field is not empty.
### Modifications
Throw `ResultAuthenticationError` when there is anything wrong with
authentication in `ClientConnection`'s constructor. Then catch the
result in `ConnectionPool::getConnectionAsync` and returned the failed
future.
In addition, check `authentication_` even for non-TLS URLs. Otherwise,
the null authentication will be used to construct `CommandConnect`.
Add `testInvalidPlugin` and `testTlsConfigError` to verify the changes.
(cherry picked from commit 7bb94f4)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Segmentation fault if failed to load an authentication plugin Test test_auth_junk_params failed after upgrading C++ client to 3.4.0

2 participants

@BewareMyPower@RobertIndie