Commit Graph

9382 Commits

Author SHA1 Message Date
Norman Maurer
707c95e80d
Use ByteProcessor in HpackHuffmanDecoder to reduce bound-checks and r… (#9317)
Motivation:

ff0045e3e1 changed HpackHuffmanDecoder to use a lookup-table which greatly improved performance. We can squeeze out another 3% win by using an ByteProcessor which will reduce the number of bound-checks / reference-count-checks needed by processing byte-by-byte.

Modifications:

Implement logic with ByteProcessor

Result:

Another ~3% perf improvement which shows up when using h2load to simulate load.

`h2load -c 100 -m 100 --duration 60 --warm-up-time 10 http://127.0.0.1:8080`

Before:

```
finished in 70.02s, 620051.67 req/s, 20.70MB/s
requests: 37203100 total, 37203100 started, 37203100 done, 37203100 succeeded, 0 failed, 0 errored, 0 timeout
status codes: 37203100 2xx, 0 3xx, 0 4xx, 0 5xx
traffic: 1.21GB (1302108500) total, 41.84MB (43872600) headers (space savings 90.00%), 460.24MB (482598600) data
                     min         max         mean         sd        +/- sd
time for request:      404us     24.52ms     15.93ms      1.45ms    87.90%
time for connect:        0us         0us         0us         0us     0.00%
time to 1st byte:        0us         0us         0us         0us     0.00%
req/s           :    6186.64     6211.60     6199.00        5.18    65.00%
```

With this change:

```
finished in 70.02s, 642103.33 req/s, 21.43MB/s
requests: 38526200 total, 38526200 started, 38526200 done, 38526200 succeeded, 0 failed, 0 errored, 0 timeout
status codes: 38526200 2xx, 0 3xx, 0 4xx, 0 5xx
traffic: 1.26GB (1348417000) total, 42.39MB (44444900) headers (space savings 90.00%), 466.25MB (488893900) data
                     min         max         mean         sd        +/- sd
time for request:      370us     24.89ms     15.52ms      1.35ms    88.02%
time for connect:        0us         0us         0us         0us     0.00%
time to 1st byte:        0us         0us         0us         0us     0.00%
req/s           :    6407.06     6435.19     6419.74        5.62    67.00%
```
2019-07-04 08:46:08 +02:00
Norman Maurer
16b98d370f
Correctly handle http2 upgrades when Http2FrameCodec is used together… (#9318)
Motivation:

In the latest release we introduced Http2MultiplexHandler as a replacement of Http2MultiplexCodec. This did split the frame parsing from the multiplexing to allow a more flexible way to handle frames and to make the code cleaner. Unfortunally we did miss to special handle this in Http2ServerUpgradeCodec and so did not correctly add Http2MultiplexHandler to the pipeline before calling Http2FrameCodec.onHttpServerUpgrade(...). This did lead to the situation that we did not correctly receive the event on the Http2MultiplexHandler and so did not correctly created the Http2StreamChannel for the upgrade stream. Because of this we ended up with an NPE if a frame was dispatched to the upgrade stream later on.

Modifications:

- Correctly add Http2MultiplexHandler to the pipeline before calling Http2FrameCodec.onHttpServerUpgrade(...)
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9314.
2019-07-04 08:32:41 +02:00
Norman Maurer
1b82474286
Fix NPE caused by re-entrance calls in FlowControlHandler (#9320)
Motivation:

2c99fc0f12 introduced a change that eagly recycles the queue. Unfortunally it did not correct protect against re-entrance which can cause a NPE.

Modifications:

- Correctly protect against re-entrance by adding null checks
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9319.
2019-07-03 19:55:18 +02:00
Carl Mastrangelo
ff0045e3e1 Use Table lookup for HPACK decoder (#9307)
Motivation:
Table based decoding is fast.

Modification:
Use table based decoding in HPACK decoder, inspired by
https://github.com/python-hyper/hpack/blob/master/hpack/huffman_table.py

This modifies the table to be based on integers, rather than 3-tuples of
bytes.  This is for two reasons:

1.  It's faster
2.  Using bytes makes the static intializer too big, and doesn't
compile.

Result:
Faster Huffman decoding.  This only seems to help the ascii case, the
other decoding is about the same.

Benchmarks:

```
Before:
Benchmark                     (limitToAscii)  (sensitive)  (size)   Mode  Cnt        Score       Error  Units
HpackDecoderBenchmark.decode            true         true   SMALL  thrpt   20   426293.636 ±  1444.843  ops/s
HpackDecoderBenchmark.decode            true         true  MEDIUM  thrpt   20    57843.738 ±   725.704  ops/s
HpackDecoderBenchmark.decode            true         true   LARGE  thrpt   20     3002.412 ±    16.998  ops/s
HpackDecoderBenchmark.decode            true        false   SMALL  thrpt   20   412339.400 ±  1128.394  ops/s
HpackDecoderBenchmark.decode            true        false  MEDIUM  thrpt   20    58226.870 ±   199.591  ops/s
HpackDecoderBenchmark.decode            true        false   LARGE  thrpt   20     3044.256 ±    10.675  ops/s
HpackDecoderBenchmark.decode           false         true   SMALL  thrpt   20  2082615.030 ±  5929.726  ops/s
HpackDecoderBenchmark.decode           false         true  MEDIUM  thrpt   10   571640.454 ± 26499.229  ops/s
HpackDecoderBenchmark.decode           false         true   LARGE  thrpt   20    92714.555 ±  2292.222  ops/s
HpackDecoderBenchmark.decode           false        false   SMALL  thrpt   20  1745872.421 ±  6788.840  ops/s
HpackDecoderBenchmark.decode           false        false  MEDIUM  thrpt   20   490420.323 ±  2455.431  ops/s
HpackDecoderBenchmark.decode           false        false   LARGE  thrpt   20    84536.200 ±   398.714  ops/s

After(bytes):
Benchmark                     (limitToAscii)  (sensitive)  (size)   Mode  Cnt        Score      Error  Units
HpackDecoderBenchmark.decode            true         true   SMALL  thrpt   20   472649.148 ± 7122.461  ops/s
HpackDecoderBenchmark.decode            true         true  MEDIUM  thrpt   20    66739.638 ±  341.607  ops/s
HpackDecoderBenchmark.decode            true         true   LARGE  thrpt   20     3139.773 ±   24.491  ops/s
HpackDecoderBenchmark.decode            true        false   SMALL  thrpt   20   466933.833 ± 4514.971  ops/s
HpackDecoderBenchmark.decode            true        false  MEDIUM  thrpt   20    66111.778 ±  568.326  ops/s
HpackDecoderBenchmark.decode            true        false   LARGE  thrpt   20     3143.619 ±    3.332  ops/s
HpackDecoderBenchmark.decode           false         true   SMALL  thrpt   20  2109995.177 ± 6203.143  ops/s
HpackDecoderBenchmark.decode           false         true  MEDIUM  thrpt   20   586026.055 ± 1578.550  ops/s
HpackDecoderBenchmark.decode           false        false   SMALL  thrpt   20  1775723.270 ± 4932.057  ops/s
HpackDecoderBenchmark.decode           false        false  MEDIUM  thrpt   20   493316.467 ± 1453.037  ops/s
HpackDecoderBenchmark.decode           false        false   LARGE  thrpt   10    85726.219 ±  402.573  ops/s

After(ints):
Benchmark                     (limitToAscii)  (sensitive)  (size)   Mode  Cnt        Score       Error  Units
HpackDecoderBenchmark.decode            true         true   SMALL  thrpt   20   615549.006 ±  5282.283  ops/s
HpackDecoderBenchmark.decode            true         true  MEDIUM  thrpt   20    86714.630 ±   654.489  ops/s
HpackDecoderBenchmark.decode            true         true   LARGE  thrpt   20     3984.439 ±    61.612  ops/s
HpackDecoderBenchmark.decode            true        false   SMALL  thrpt   20   602489.337 ±  5397.024  ops/s
HpackDecoderBenchmark.decode            true        false  MEDIUM  thrpt   20    88399.109 ±   241.115  ops/s
HpackDecoderBenchmark.decode            true        false   LARGE  thrpt   20     3875.729 ±   103.057  ops/s
HpackDecoderBenchmark.decode           false         true   SMALL  thrpt   20  2092165.454 ± 11918.859  ops/s
HpackDecoderBenchmark.decode           false         true  MEDIUM  thrpt   20   583465.437 ±  5452.115  ops/s
HpackDecoderBenchmark.decode           false         true   LARGE  thrpt   20    93290.061 ±   665.904  ops/s
HpackDecoderBenchmark.decode           false        false   SMALL  thrpt   20  1758402.495 ± 14677.438  ops/s
HpackDecoderBenchmark.decode           false        false  MEDIUM  thrpt   10   491598.099 ±  5029.698  ops/s
HpackDecoderBenchmark.decode           false        false   LARGE  thrpt   20    85834.290 ±   554.915  ops/s
```
2019-07-02 20:09:44 +02:00
秦世成
18e4121952 Pre-decompressed DNS record RData that may contain compression pointers (#9311)
Motivation:

When decoding DnsRecord, if the record contains compression pointers, and not all compression pointers are decompressed, but part of the pointers are decompressed. Then when encoding the record, the compressed pointer will point to the wrong location, resulting in bad label problem.

Modification:

Pre-decompressed record RData that may contain compression pointers.

Result:

Fixes #8962
2019-07-02 19:38:50 +02:00
Carl Mastrangelo
deea51e609 Disable Huffman encoding for small headers (#9260)
Motivation:

Huffman coding saves only a little space, but has a huge CPU cost

Modification:

Disable huff coding for headers smaller than 512 bytes.  Also, add a
configurable limit to the encoder.

Result:

Faster HPACK

BEFORE:
```
Benchmark                     (duplicates)  (limitToAscii)  (sensitive)  (size)  Mode  Cnt       Score       Error  Units
HpackEncoderBenchmark.encode          true            true         true   SMALL  avgt   10    2572.595 ±    16.184  ns/op
HpackEncoderBenchmark.encode          true            true         true  MEDIUM  avgt   10   19580.815 ±   397.780  ns/op
HpackEncoderBenchmark.encode          true            true         true   LARGE  avgt   10  379456.381 ±  2059.919  ns/op
HpackEncoderBenchmark.encode          true            true        false   SMALL  avgt   10     730.579 ±     8.116  ns/op
HpackEncoderBenchmark.encode          true            true        false  MEDIUM  avgt   10    2087.590 ±    84.644  ns/op
HpackEncoderBenchmark.encode          true            true        false   LARGE  avgt   10   11725.228 ±    89.298  ns/op
HpackEncoderBenchmark.encode          true           false         true   SMALL  avgt   10     555.971 ±     5.120  ns/op
HpackEncoderBenchmark.encode          true           false         true  MEDIUM  avgt   10    2831.874 ±    41.801  ns/op
HpackEncoderBenchmark.encode          true           false         true   LARGE  avgt   10   36054.025 ±   179.504  ns/op
HpackEncoderBenchmark.encode          true           false        false   SMALL  avgt   10     340.337 ±     3.313  ns/op
HpackEncoderBenchmark.encode          true           false        false  MEDIUM  avgt   10    1006.817 ±     8.942  ns/op
HpackEncoderBenchmark.encode          true           false        false   LARGE  avgt   10    8784.168 ±   164.014  ns/op
HpackEncoderBenchmark.encode         false            true         true   SMALL  avgt   10    2561.934 ±    27.056  ns/op
HpackEncoderBenchmark.encode         false            true         true  MEDIUM  avgt   10   22061.105 ±   154.533  ns/op
HpackEncoderBenchmark.encode         false            true         true   LARGE  avgt   10  435744.897 ±  8853.388  ns/op
HpackEncoderBenchmark.encode         false            true        false   SMALL  avgt   10    2737.683 ±    47.142  ns/op
HpackEncoderBenchmark.encode         false            true        false  MEDIUM  avgt   10   22385.146 ±    98.430  ns/op
HpackEncoderBenchmark.encode         false            true        false   LARGE  avgt   10  408159.698 ± 12044.931  ns/op
HpackEncoderBenchmark.encode         false           false         true   SMALL  avgt   10     544.213 ±     3.279  ns/op
HpackEncoderBenchmark.encode         false           false         true  MEDIUM  avgt   10    2908.978 ±    31.026  ns/op
HpackEncoderBenchmark.encode         false           false         true   LARGE  avgt   10   36471.262 ±  1044.010  ns/op
HpackEncoderBenchmark.encode         false           false        false   SMALL  avgt   10     609.305 ±     4.371  ns/op
HpackEncoderBenchmark.encode         false           false        false  MEDIUM  avgt   10    3223.946 ±    23.505  ns/op
HpackEncoderBenchmark.encode         false           false        false   LARGE  avgt   10   39975.152 ±   655.196  ns/op
```

AFTER:
```
NEW AFTER

Benchmark                     (duplicates)  (limitToAscii)  (sensitive)  (size)  Mode  Cnt     Score     Error  Units
HpackEncoderBenchmark.encode          true            true         true   SMALL  avgt    5   379.473 ± 133.815  ns/op
HpackEncoderBenchmark.encode          true            true         true  MEDIUM  avgt    5  1118.772 ±  89.258  ns/op
HpackEncoderBenchmark.encode          true            true         true   LARGE  avgt    5  5366.828 ±  89.746  ns/op
HpackEncoderBenchmark.encode          true            true        false   SMALL  avgt    5   284.401 ±   2.088  ns/op
HpackEncoderBenchmark.encode          true            true        false  MEDIUM  avgt    5   922.805 ±  10.796  ns/op
HpackEncoderBenchmark.encode          true            true        false   LARGE  avgt    5  8727.831 ± 462.138  ns/op
HpackEncoderBenchmark.encode          true           false         true   SMALL  avgt    5   337.093 ±  22.585  ns/op
HpackEncoderBenchmark.encode          true           false         true  MEDIUM  avgt    5   693.689 ±  16.351  ns/op
HpackEncoderBenchmark.encode          true           false         true   LARGE  avgt    5  5616.786 ±  98.647  ns/op
HpackEncoderBenchmark.encode          true           false        false   SMALL  avgt    5   286.708 ±  13.765  ns/op
HpackEncoderBenchmark.encode          true           false        false  MEDIUM  avgt    5   906.279 ±  32.338  ns/op
HpackEncoderBenchmark.encode          true           false        false   LARGE  avgt    5  8304.736 ± 128.584  ns/op
HpackEncoderBenchmark.encode         false            true         true   SMALL  avgt    5   351.381 ±  15.547  ns/op
HpackEncoderBenchmark.encode         false            true         true  MEDIUM  avgt    5  1188.166 ±   7.023  ns/op
HpackEncoderBenchmark.encode         false            true         true   LARGE  avgt    5  6876.009 ±  48.117  ns/op
HpackEncoderBenchmark.encode         false            true        false   SMALL  avgt    5   434.759 ±   8.619  ns/op
HpackEncoderBenchmark.encode         false            true        false  MEDIUM  avgt    5   954.588 ±  58.514  ns/op
HpackEncoderBenchmark.encode         false            true        false   LARGE  avgt    5  8534.017 ± 552.597  ns/op
HpackEncoderBenchmark.encode         false           false         true   SMALL  avgt    5   223.713 ±   4.823  ns/op
HpackEncoderBenchmark.encode         false           false         true  MEDIUM  avgt    5  1181.538 ±  11.851  ns/op
HpackEncoderBenchmark.encode         false           false         true   LARGE  avgt    5  6670.830 ± 267.927  ns/op
HpackEncoderBenchmark.encode         false           false        false   SMALL  avgt    5   424.609 ±  27.477  ns/op
HpackEncoderBenchmark.encode         false           false        false  MEDIUM  avgt    5  1003.578 ±  53.991  ns/op
HpackEncoderBenchmark.encode         false           false        false   LARGE  avgt    5  8428.932 ± 102.838  ns/op
```
2019-07-01 21:00:20 +02:00
Norman Maurer
131be58f48
Correctly take length of ByteBufInputStream into account for readLine… (#9310)
* Correctly take length of ByteBufInputStream into account for readLine() / readByte()

Motivation:

ByteBufInputStream did not correctly take the length into account when validate bounds for readLine() / readByte() which could lead to read more then allowed.

Modifications:

- Correctly take length into account
- Add unit tests
- Fix existing unit test

Result:

Correctly take length of ByteBufInputStream into account.
Related to https://github.com/netty/netty/pull/9306.
2019-07-01 20:55:23 +02:00
Dmitriy Dumanskiy
5ded050f7b #7285 Improved "Discarded inbound message" warning (#9286)
Motivation:

On servers with many pipelines or dynamic pipelines, it is easy for end user to make mistake during pipeline configuration. Current message:

`Discarded inbound message PooledUnsafeDirectByteBuf(ridx: 0, widx: 2, cap: 2) that reached at the tail of the pipeline. Please check your pipeline configuration.`

Is not always meaningful and doesn't allow to find the wrong pipeline quickly.

Modification:

Added additional log placeholder that identifies pipeline handlers and channel info. This will allow for the end users quickly find the problem pipeline.

Result:

Meaningful warning when the message reaches the end of the pipeline. Fixes #7285
2019-07-01 20:38:58 +02:00
xiaoheng1
f8c1f350db Fix public int read() throws IOException method exceeds the limit of length (#9306)
Motivation:

buffer.isReadable() should not be used to limit the amount of data that can be read as the amount may be less then was is readable.

Modification:

- Use  available() which takes the length into account
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9305
2019-07-01 15:57:34 +02:00
Cory Benfield
14154074f2 Don't loop over TLS records for SNI (#7479)
Motivation:

The AbstractSniHandler previously was willing to tolerate up to three
non-handshake records before a ClientHello that contained an SNI
extension field. This is, so far as I can tell, completely
unnecessary: no TLS implementation will be sending alerts or change
cipher spec messages before ClientHello.

Given that it was not possible to determine why this loop is in
the code to begin with, it's probably just best to remove it.

Modifications:

Remove the for loop.

Result:

The AbstractSniHandler will more rapidly determine whether it should
pass the records on to the default SSL handler.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2019-07-01 11:22:55 +02:00
秦世成
4596f9e139 Fix the issue of incorrectly calculating the number of dump lines when using PrettyDump in ByteBufUtil (#9304)
Motivation:

Fix the issue of incorrectly calculating the number of dump rows when using prettyHexDumpmethod in ByteBufUtil. The way to find the remainder is either length % 16 or length & 15

Modification:

Fixed the way to calculate the remainder

Result:

Fixed #9301
2019-07-01 08:35:18 +02:00
Carl Mastrangelo
262ced7ce4 Unconditionally initialize sockaddrs in epoll linuxsocket (#9299)
Motivation:

Compiling with -Werror,-Wuninitialized complains about the sockaddrs being uninitialized.
I believe this is because the init function netty_unix_socket_initSockaddr is in a
separate compilation unit.  Since this code isn't on the criticial path, it's easy
to just memset the variables rather than suppress the warning.

Modification:
Always clear the sockaddrs, even if they will be initialized later.

Result:
Able to compile with warnings turned on
2019-06-29 12:16:58 +02:00
Norman Maurer
05c2967e4a
Http2FrameCodecBuilder.autoAckSettingsFrame(...) must be public (#9295)
Motivation:

b3dba317d7 added AbstractHttp2ConnectionBuilder.autoAckSettingsFrame(...) as protected method and made it public for Http2MultiplexCodecBuilder. Unfortunally it did miss to also make it public in Http2FrameCodecBuilder

Modifications:

Correctly override autoAckSettingsFrame in Http2FrameCodecBuilder and so make it usable when building Http2FrameCodec.

Result:

Be able to also configure autoAckSettingsFrame when Http2FrameCodec is used.
2019-06-29 09:23:38 +02:00
Norman Maurer
47eb9c3bf4
Ensure sc.close() is executed before FixedChannelPoolTest.testCloseAsync() returns (#9298)
Motivation:

We observed some test-failues sometimes in the CI which happened if sc.close() was not completed before the next test did run. If this happened we would fail the bind(...) as the LocalAddress was still in use.

Modifications:

Await the close before return

Result:

Fixes race in testsuite which resulted in FixedChannelPoolTest.testAcquireNewConnection to fail if FixedChannelPoolTest.testCloseAsync() did run before it.
2019-06-29 09:21:11 +02:00
jimin
ee8206cb26 optimize some code (#9289)
Motivation:

There is some manual coping of elements of Collections which can be replaced by Collections.addAll(...) and also some unnecessary semicolons.

Modifications:

- Simplify branches
- Use Collections.addAll
- Code cleanup

Result:

Code cleanup
2019-06-28 13:48:23 +02:00
Aleksey Yeschenko
6b6475fb56 Prevent ByteToMessageDecoder from overreading when !isAutoRead (#9252)
Motivation:

ByteToMessageDecoder only looks at the last channelRead() in the batch
of channelRead()-s when determining whether or not it should call
ChannelHandlerContext#read() to consume more data when !isAutoRead. This
will lead to read() calls issued unnecessaily and unprompted if the very
last channelRead() didn't result in at least one decoded message, even
if there have been messages decoded from other channelRead()-s in the
current batch.

Modifications:

Track decode outcomes for the entire batch of channelRead() calls and
only issue a read in BTMD if the entire batch of channelRead() calls
yielded no complete messages.

Result:

ByteToMessageDecoder will no longer overread when the very last read
yielded no message, but the batch of reads did.
2019-06-28 13:43:25 +02:00
Farid Zakaria
efe40ac17d Add a test for OpenSslEngine which decrypts traffic (#8699)
Motivation:
I've introduced netty/netty-tcnative#421 that introduced exposing OpenSSL master key & client/server
random values with the purpose of allowing someone to log them to debug the traffic via auxiliary tools like Wireshark (see also #8653)

Modification:
Augmented OpenSslEngineTest to include a test which manually decrypts the TLS ciphertext
after exposing the masterkey + client/server random. This acts as proof that the tc-native new methods work correctly!

Result:

More tests

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
2019-06-28 13:41:24 +02:00
root
5b58b8e6b5 [maven-release-plugin] prepare for next development iteration 2019-06-28 05:57:21 +00:00
root
35e0843376 [maven-release-plugin] prepare release netty-4.1.37.Final 2019-06-28 05:56:28 +00:00
秦世成
f489404fa1 HAProxyMessageDecoder not correctly handle delimiter in all cases (#9282)
Motivation:

In line base decoders, lines are split by delimiter, but the delimiter may be \r\n or \r, so in decoding, if findEndOfLine finds delimiter of a line, the length of the delimiter may be 1 or 2, instead of DELIMITER_LENGTH, where the value is fixed to 2.
The second problem is that if the data to be decoded is too long, the decoder will discard too long data, and needs to record the length of the discarded bytes. In the original implementation, the discarded bytes are not accumulated, but are assigned to the currently discarded bytes.
Modification:

Modifications:
Dynamic calculation of the length of delimiter.
In discarding mode, add up the number of characters discarded each time.

Result:

Correctly handle all delimiters and also correctly handle too long frames.
2019-06-27 21:58:46 +02:00
jimin
51843d8e8e MqttConnectPayload.toString() should use Arrays.toString() instead of [].toString() (#9292)
Motivation:

The toString() method should use Arrays.toString() to produce a meaningful String representation for arrays.

Modification:

Use Arrays.toString()

Result:

More useful toString() implementation
2019-06-27 21:55:02 +02:00
Norman Maurer
7dff856b1f
Don't propagate Http2WindowUpdateFrame to the child channel / propagate Http2ResetFrame as user event when using Http2MultiplexHandler (#9290)
Motivation:

We should not propage Http2WindowUpdateFrames to the child channels at all as these are not really use-ful and should not be flow-controlled via `read()` anyway.  In the other hand Http2ResetFrame is very useful but should be propagated via an user event so the user is aware of it directly even if the user stops reading.

Modifications:

- Dont propagate Http2WindowUpdateFrames when using Http2MultiplexHandler
- Use user event for Http2ResetFrame when using Http2MultiplexHandler
- Adjust javadoc of Http2MultiplexHandler
- Add unit tests

Result:

Fixes https://github.com/netty/netty/pull/8889 and https://github.com/netty/netty/pull/7635
2019-06-27 21:52:52 +02:00
Norman Maurer
df46a349e0
Reduce coupeling between Http2FrameCodec and Http2Multiplex* (#9273)
Motivation:

Http2MultiplexCodec and Http2MultiplexHandler had a very strong coupling with Http2FrameCodec which we can reduce easily. The end-goal should be to have no coupling at all.

Modifications:

- Reduce coupling by move some common logic to Http2CodecUtil
- Move logic to check if a stream may have existed before to Http2FrameCodec
- Use ArrayDeque as replacement for custom double-linked-list which makes the code a lot more readable
- Use WindowUpdateFrame to signal consume bytes (just as users do when they use Http2FrameCodec directly)

Result:

Less coupling and cleaner code.
2019-06-27 21:43:31 +02:00
jimin
856f1185e1 All override methods must be added @override (#9285)
Motivation:

Some methods that either override others or are implemented as part of implementation an interface did miss the `@Override` annotation

Modifications:

Add missing `@Override`s

Result:

Code cleanup
2019-06-27 13:51:26 +02:00
jimin
9621a5b981 remove unused imports (#9287)
Motivation:

Some imports are not used

Modification:

remove unused imports

Result:

Code cleanup
2019-06-26 21:08:31 +02:00
jimin
6bd8f0502d Call to ‘asList’ with only one argument could be replaced with ‘singletonList’ (#9288)
Motivation:

asList should only be used if there are multiple elements.

Modification:

Call to asList with only one argument could be replaced with singletonList

Result:

Cleaner code and a bit of memory savings
2019-06-26 21:06:48 +02:00
Alex Blewitt
52169cba95 Replace accumulation with blackhole.consume (#9275)
Motivation:

SpotJMHBugs reports that accumulating a value as a way of eliding dead code
elimination may be inadvisable, as discussed in
`JMHSample_34_SafeLooping::measureWrong_2`. Change the test so that it consumes
the response with `Blackhole::consume` instead.

Modifications:

- Replace addition of results with explicit `blackhole.consume()` call

Result:

Tests work as before, but with different benchmark numbers.
2019-06-25 21:47:07 +02:00
Francesco Nigro
672fa0c779 Documented non-usage of BlackHole::consume on ByteBufAccessBenchmark (#9279)
Motivation:

Some JMH benchmarks need additional explanations to motivate
specific code choices.

Modifications:

Introduced comment to explai why calling BlackHole::consume
in a loop is not always the right choice for some benchmark.

Result:

The relevant method shows a comment that warn about changing
the code to introduce BlackHole::consume in the loop.
2019-06-25 14:52:21 +02:00
Graeme Rocher
18b7bdff12 Add null check to isSkippable. Fixes #9278 (#9280)
Motivation:

Currently GraalVM substrate returns null for reflective calls if the reflection access is not declared up front.

A change introduced in Netty 4.1.35 results in needing to register every Netty handler for reflection. This complicates matters as it is difficult to know all the possible handlers that need to be registered.

Modification:

This change adds a simple
null check such that Netty does not break on GraalVM substrate without the reflection information registration.

Result:

Fixes #9278
2019-06-25 14:45:30 +02:00
Stephane Landelle
039087ed47 Don't filter out TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (#9274)
Motivation:

TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 is supported since Java 8 (see https://docs.oracle.com/javase/8/docs/technotes/guides/security/SunProviders.html) and belongs to the recommended configurations in many references, eg SSLabs (https://github.com/ssllabs/research/wiki/SSL-and-TLS-Deployment-Best-Practices) or Google Cloud Platform Restricted Profile.

Modifications:

Add TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 to default ciphers list.

Result:

TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 is enabled by default.
2019-06-24 23:11:24 +02:00
Norman Maurer
265c745d9a
EmptyByteBuf.getCharSequence(0,...) must return empty String (#9272)
Motivation:

At the moment EmptyByteBuf.getCharSequence(0,...) will return null while it must return a "".

Modifications:

- Let EmptyByteBuf.getCharSequence(0,...) return ""
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9271.
2019-06-24 21:09:19 +02:00
Norman Maurer
097f422198
Cleanup http2 example code to make clear it is fine to just use ctx directly. (#9276)
Motivation:

In our example we did use pipeline.context(this) to obtain the context of the handler while it was already passed in via ctx. This could confuse users and give the impression that the context is no the same.

Modifications:

Just use ctx directly.

Result:

Fix confusion in example code. This was brought up on stackoverflow:

https://stackoverflow.com/questions/56711128/when-is-a-channelhandlercontext-handed-to-a-channelhandler-not-that-channelhandl
2019-06-24 21:08:02 +02:00
Julien Viet
1ad47282c3 Preserve the original filename when encoding a multipart/form in mixed mode. (#9270)
Motivation:

The HttpPostRequestEncoder overwrites the original filename of file uploads sharing the same name encoded in mixed mode when it rewrites the multipart body header of the previous file. The original filename should be preserved instead.

Modifications:

Change the HttpPostRequestEncoder to reuse the correct filename when the encoder switches to mixed mode. The original test is incorrect and has been modified too, in addition it tests with an extra file upload since the current test was not testing the continuation of a mixed mode.

Result:

The HttpPostRequestEncoder will preserve the original filename of the first fileupload when switching to mixed mode
2019-06-24 10:40:17 +02:00
秦世成
712077cdef Fixed the haproxy message mem leak issue (#9250)
Motivation:

HAProxyMessage should be released as it contains a list of TLV which hold a ByteBuf, otherwise, it may cause memory leaks.

Modification:

- Let HAProxyMessage extend AbstractReferenceCounted
- Adjust tests.

Result:

Fixes #9201
2019-06-24 10:38:58 +02:00
Norman Maurer
307efbe49c
Split multiplexing from frame decoding to allow easier customization of frame processing and better seperation of responsibilities (#9239)
Motivation:

In the past we had the following class hierarchy:

Http2ConnectionHandler --- Http2FrameCodec -- Http2MultiplexCodec

This hierarchy makes it impossible to plug in any code that would like to act on Http2Frame and Http2StreamFrame which can be quite useful for various situations (like metrics, logging etc). Beside this it also made the implementtion very hacky. To allow easier maintainance and also allow more flexible costumizations we should split Http2MultiplexCodec and Http2FrameCode.

Modifications:

- Introduce Http2MultiplexHandler (which is a replacement for Http2MultiplexCodec when used together with Http2FrameCodec)
- Mark Http2MultiplexCodecBuilder and Http2MultiplexCodec as deprecated. People should use Http2FrameCodecBuilder / Http2FrameCodec together with Http2MultiplexHandlder in the future
- Adjust / Add tests
- Adjust examples

Result:

More flexible usage possible and less hacky / coupled implementation for http2 multiplexing
2019-06-24 09:17:15 +02:00
ursa
41c1ab2e82 Bugfix #9257: WebSocketProtocolHandler does NOT support autoRead=false (#9258)
Motivation:

I need to control WebSockets inbound flow manually, when autoRead=false

Modification:

Add missed ctx.read() call into WebSocketProtocolHandler, where read request has been swallowed.

Result:

Fixes #9257
2019-06-24 09:07:57 +02:00
Norman Maurer
517a93d87d Make EventLoopTaskQueueFactory a top-level interface
Motivation:

c9aaa93d83 added the ability to specify an EventLoopTaskQueueFactory but did place it under MultithreadEventLoopGroup while not really belongs there.

Modifications:

Make EventLoopTaskQueueFactory a top-level interface

Result:

More logical code layout.
2019-06-22 07:38:03 +02:00
Norman Maurer
2c99fc0f12
Recycle RecyclableArrayDeque as fast as possible in FlowControlHandler (#9263)
Motivation:

FlowControlHandler does use a recyclable ArrayDeque internally but only recycles it when the channel is closed. We should better recycle it once it is empty.

Modifications:

Recycle the deque as fast as possible

Result:

Less RecyclableArrayDeque instances.
2019-06-22 07:27:04 +02:00
Alex Blewitt
430eeee2f6 Return the result of the list.recycle() call (#9264)
Motivation:

Resolve the issue highlighted by SpotJMHBugs that the creation of the RecyclableArrayList may be elided by the JIT since the result isn't consumed or returned.

Modifications:

Return the result of `list.recycle()` so that the list isn't elided.

Result:

The JMH benchmark shows a change in performance indicating that the prior results of this may be unsound.
2019-06-22 07:22:15 +02:00
Nick Hill
2af769f6dc Subsequence versions of ByteBufUtil#writeUtf8(...) methods (#9224)
Motivation

It would be useful to be able to write UTF-8 encoded subsequence of
CharSequence characters to a ByteBuf without needing to create a
temporary object via CharSequence#subSequence().

Modification

Add overloads of ByteBufUtil writeUtf8, reserveAndWriteUtf8 and
utf8Bytes methods which take explicit subsequence bounds.

Result

More efficient writing of substrings to byte buffers possible
2019-06-21 14:05:35 +02:00
Norman Maurer
9dd1aab482
Fix flaky DnsNameResolverTest.testTruncatedWithTcpFallback (#9262)
Motivation:

testTruncatedWithTcpFallback was flacky as we may end up closing the socket before we could read all data. We should only close the socket after we succesfully read all data.

Modifications:

Move socket.close() to finally block

Result:

Fix flaky test and so make the CI more stable again.
2019-06-21 09:28:51 +02:00
Norman Maurer
c9aaa93d83
Allow to specify a EventLoopTaskQueueFactory for various EventLoopGroup implementations (#9247)
Motivation:

Sometimes it is desirable to be able to use a different Queue implementation for the EventLoop of a Channel. This is currently not possible without resort to reflection.

Modifications:

- Add a new constructor to Nio|Epoll|KQueueEventLoopGroup which allows to specify a factory which is used to create the task queue. This was the user can override the default implementation.
- Add test

Result:

Be able to change Queue that is used for the EventLoop.
2019-06-21 09:05:19 +02:00
Nick Hill
6381d0766a De-duplicate PooledByteBuf implementations (#9120)
Motivation

There's quite a lot of duplicate/equivalent logic across the various
concrete ByteBuf implementations. We could take this even further but
for now I've focused on the PooledByteBuf sub-hierarchy.

Modifications

- Move common logic/methods into existing PooledByteBuf abstract
superclass
- Shorten PooledByteBuf.capacity(int) method implementation

Result

Less code to maintain
2019-06-19 20:50:27 +02:00
Kevin Oliver
c32c9b4c94 codec-http2: Lazily translate cookies for HTTP/1 (#9251)
Motivation:

For HTTP/2 messages with multiple cookies HttpConversionUtil.addHttp2ToHttpHeaders spends a good portion of time creating throwaway StringBuilders.

Modification:

Handle cookies lazily by using a ThreadLocal StringBuilder and then converting it to the H1 header at the end.

Result:

Less allocations.
2019-06-19 11:03:49 +02:00
Norman Maurer
01cfd78d6d
Try to mark child channel writable again once the parent channel becomes writable (#9254)
Motivation:

f945a071db decoupled the writability state from the flow controller but could lead to the situation of a lot of writability updates events were propagated to the child channels. This change ensure we only take into account if the parent channel becomes writable again before we try to set the child channels to writable.

Modifications:

Only listen for channel writability changes for if the parent channel becomes writable again.

Result:

Less writability updates.
2019-06-18 20:30:31 +02:00
ursa
7fc718db3c WebSocket is closed without an error on protocol violations (#9116)
Motivation:

Incorrect WebSockets closure affects our production system.
Enforced 'close socket on any protocol violation' prevents our custom termination sequence from execution.
Huge number of parameters is a nightmare both in usage and in support (decoders configuration).
Modification:

Fix violations handling - send proper response codes.
Fix for messages leak.
Introduce decoder's option to disable default behavior (send close frame) on protocol violations.
Encapsulate WebSocket response codes - WebSocketCloseStatus.
Encapsulate decoder's configuration into a separate class - WebSocketDecoderConfig.
Result:

Fixes #8295.
2019-06-18 10:05:58 +02:00
Norman Maurer
f945a071db
Writability state of http2 child channels should be decoupled from the flow-controller (#9235)
Motivation:

We should decouple the writability state of the http2 child channels from the flow-controller and just tie it to its own pending bytes counter that is decremented by the parent Channel once the bytes were written.

Modifications:

- Decouple writability state of child channels from flow-contoller
- Update tests

Result:

Less coupling and more correct behavior. Fixes https://github.com/netty/netty/issues/8148.
2019-06-18 09:37:59 +02:00
Frédéric Brégier
b1fb40e42d Change Scheduled to FixedRate in Traffic Counter (#9245)
Motivation:

Traffic shaping needs more accurate execution than scheduled one. So the
use of FixedRate instead.
Moreover the current implementation tends to create as many threads as
channels use a ChannelTrafficShapingHandlern, which is unnecessary.

Modifications:

Change the executor.schedule to executor.scheduleAtFixedRate in the
start and remove the reschedule call from run monitor thread since it
will be restarted by the Fixed rate executor.
Also fix a minor bug where restart was only doing start() without stop()
before.

Result:

Threads are more stable in number of cached and precision of traffic
shaping is enhanced.
2019-06-18 09:34:48 +02:00
Aleksey Yeschenko
93414db1f3 Fix LZ4 encoder/decoder performance with (default) xxHash32 (#9249)
Motivation:

Lz4FrameEncoder and Lz4FrameDecoder in their default configuration use
an extremely inefficient way to checksum direct byte buffers. In
particular, for every byte checksummed, a single-element byte array is
being allocated and a JNI cal is made, which in some internal testing
makes a 25x difference in total throughput and allocates *a lot* of
garbage.

Modifications:

Lz4XXHash32, an implementation of ByteBufChecksum specifically for use
by Lz4FrameEncoder and Lz4FrameDecoder, is introduced. It utilises
xxHash32 block API which provides a hash() method that accepts a
ByteBuffer as an argument. Lz4FrameEncoder and Lz4FrameDecoder are
modified to use this implementation by default.

Result:

Lz4FrameEncoder and Lz4FrameDecoder perform well again when operating
on direct byte buffers with default checksum configuration; a public
implementation is provided for those who need to override the seed.
2019-06-18 09:29:25 +02:00
Aleksey Yeschenko
a2583d0d3c Fix ReflectiveByteBufChecksum with direct buffers (#9244)
Motivation:

ReflectiveByteBufChecksum#update(buf, off, len) ignores provided offset
and length arguments when operating on direct buffers, leading to wrong
byte sequences being checksummed and ultimately incorrect checksum
values (unless checksumming the entire buffer).

Modifications:

Use the provided offset and length arguments to get the correct nio
buffer to checksum; add test coverage exercising the four meaningfully
different offset and length combinations.

Result:

Offset and length are respected and a correct checksum gets calculated;
simple unit test should prevent regressions in the future.
2019-06-17 16:32:12 +02:00