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.

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

Fix ProxyProvider hash function leading to memory leakage #728

Closed
wants to merge 1 commit into from

Conversation

shrykull
Copy link

@shrykull shrykull commented May 2, 2019

it seems i triggered a memory leak by registering a proxy with reactor.netty.http.client.HttpClient.create().tcpConfiguration(this::configureProxy) as the lambda is called for every connection due to the ChannelHandler being a "different" one in every call of reactor/netty/resources/PooledConnectionProvider.java:137

i guess the problem is that the used hash function reactor.netty.tcp.ProxyProvider#hashCode includes the nonProxyHosts as a regex pattern which gets his hash function from java.lang.Object#hashCode and therefore fails to recognize equal regexes. The tests in src/test/java/reactor/netty/tcp/ProxyProviderTest.java did not break because they get null as test fixture for nonProxyHosts.

This ultimately causes the reactor.netty.resources.PooledConnectionProvider#channelPools to grow indefinitely once a proxy with defined nonProxyHosts is used, no matter if the connection is actually proxied or not.

@pivotal-issuemaster
Copy link

@shrykull Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@shrykull Thank you for signing the Contributor License Agreement!

@codecov-io
Copy link

Codecov Report

Merging #728 into master will decrease coverage by 0.07%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #728      +/-   ##
===========================================
- Coverage     65.87%   65.8%   -0.08%     
- Complexity     1397    1398       +1     
===========================================
  Files           130     130              
  Lines          6596    6597       +1     
  Branches        891     891              
===========================================
- Hits           4345    4341       -4     
- Misses         1758    1763       +5     
  Partials        493     493
Impacted Files Coverage Δ Complexity Δ
src/main/java/reactor/netty/tcp/ProxyProvider.java 68.42% <66.66%> (+0.27%) 26 <2> (+2) ⬆️
...ctor/netty/resources/PooledConnectionProvider.java 78.16% <0%> (-1.41%) 22% <0%> (ø)
...eactor/netty/channel/ChannelOperationsHandler.java 70.83% <0%> (-0.22%) 65% <0%> (-1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ed1431...f3f75b9. Read the comment docs.

@violetagg violetagg changed the base branch from master to 0.8.x May 3, 2019 10:51
@violetagg violetagg changed the base branch from 0.8.x to master May 3, 2019 10:52
@violetagg violetagg added this to the 0.8.7.RELEASE milestone May 3, 2019
@violetagg violetagg added type/bug A general bug and removed type/bug A general bug labels May 3, 2019
@violetagg
Copy link
Member

Fix with e39d2c1

@violetagg violetagg closed this May 3, 2019
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.

None yet

4 participants