FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Support for OpenSslEngine with no finalizer by johnou · Pull Request #1695 · AsyncHttpClient/async-http-client · GitHub

Support for OpenSslEngine with no finalizer - #1695

Open
johnou wants to merge 1 commit into
AsyncHttpClient:mainfrom
johnou:bugfix/ssl_context_release
Open

Support for OpenSslEngine with no finalizer#1695
johnou wants to merge 1 commit into
AsyncHttpClient:mainfrom
johnou:bugfix/ssl_context_release

Conversation

johnou commented Feb 5, 2020

Copy link
Copy Markdown
Contributor

Motivation:

Custom SslContext should be retained on init.

Modification:

Retain custom SslContext.

Result:

Externally managed SslContext refcount is managed correctly.

Motivation:

Custom SslContext should be retained on init.

Modification:

Retain custom SslContext.

Result:

Externally managed SslContext refcount is managed correctly.

johnou commented Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

Follow up for #1669

@normanmaurer ptal

Copy link
Copy Markdown
Contributor

Why? In this case, SslContext is provided by the user so he has control over ref count. Moreover, isn’t it created with a refcount of 1?

johnou commented Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

@slandelle yep but then it is decreased in destroy. point is user should know if they haven't cleaned up any resources, so in the case we are dealing with an SslContext provided by the user, refcount will be incremented to 2, then decremented back to 1 once destroy is invoked on the SslEngineFactory.

johnou commented Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

I don't think the test failures are related..

Tests run: 15, Failures: 6, Errors: 0, Skipped: 0, Time elapsed: 4.185 sec <<< FAILURE! - in TestSuite
testAbort(org.asynchttpclient.extras.rxjava.single.AsyncHttpSingleTest)  Time elapsed: 2.447 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.single.AsyncHttpSingleTest.testAbort(AsyncHttpSingleTest.java:282)
Caused by: java.net.UnknownHostException: DNS name not found [response code 3]
	at org.asynchttpclient.extras.rxjava.single.AsyncHttpSingleTest.testAbort(AsyncHttpSingleTest.java:277)

testObserveError(org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest)  Time elapsed: 0.049 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveError(AsyncHttpObservableTest.java:98)
Caused by: java.net.UnknownHostException: gatling.io
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveError(AsyncHttpObservableTest.java:94)

testObserveMultiple(org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest)  Time elapsed: 0.2 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveMultiple(AsyncHttpObservableTest.java:121)
Caused by: java.net.UnknownHostException: gatling.io
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveMultiple(AsyncHttpObservableTest.java:114)

testObserveNoError(org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest)  Time elapsed: 0.054 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveNoError(AsyncHttpObservableTest.java:78)
Caused by: java.net.UnknownHostException: gatling.io
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testObserveNoError(AsyncHttpObservableTest.java:74)

testToObservableError(org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest)  Time elapsed: 0.03 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testToObservableError(AsyncHttpObservableTest.java:58)
Caused by: java.net.UnknownHostException: gatling.io
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testToObservableError(AsyncHttpObservableTest.java:55)

testToObservableNoError(org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest)  Time elapsed: 0.026 sec  <<< FAILURE!
java.lang.AssertionError: Unexpected onError events (0 completions) (+1 error)
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testToObservableNoError(AsyncHttpObservableTest.java:38)
Caused by: java.net.UnknownHostException: gatling.io
	at org.asynchttpclient.extras.rxjava.AsyncHttpObservableTest.testToObservableNoError(AsyncHttpObservableTest.java:35)


Results :

Failed tests: 
  AsyncHttpObservableTest.testObserveError:98 Unexpected onError events (0 completions) (+1 error)
  AsyncHttpObservableTest.testObserveMultiple:121 Unexpected onError events (0 completions) (+1 error)
  AsyncHttpObservableTest.testObserveNoError:78 Unexpected onError events (0 completions) (+1 error)
  AsyncHttpObservableTest.testToObservableError:58 Unexpected onError events (0 completions) (+1 error)
  AsyncHttpObservableTest.testToObservableNoError:38 Unexpected onError events (0 completions) (+1 error)
  AsyncHttpSingleTest.testAbort:282 Unexpected onError events (0 completions) (+1 error)

johnou commented Feb 5, 2020

Copy link
Copy Markdown
Contributor Author

The other possible solution is something like this.. https://gist.github.com/johnou/4c699909ef9be3c030eaee29432c1ce3#file-defaultsslenginefactory-java-L34

slandelle force-pushed the master branch 5 times, most recently from 6ea11f4 to f8fab66 Compare February 7, 2020 12:25
hyperxpro force-pushed the main branch 2 times, most recently from c6640bd to 83e552e Compare August 9, 2026 22:30
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL