Skip to content

[Bug] Triple REST does not clean up HttpPostRequestDecoder after a form request completes #16403

Description

@juzi050

Pre-check

  • I am sure that all the content I provide is in English.

Search before asking

  • I had searched in the issues and found no similar issues.

Apache Dubbo Component

Java SDK (apache/dubbo)

Dubbo Version

Dubbo: 3.3.7-SNAPSHOT
Branch: 3.3
Commit: 3a30432
JDK: Eclipse Temurin 17.0.17
Operating system: Windows 11 x86_64
Netty: 4.2.15.Final

Steps to reproduce this issue

Add the following test to the existing RestProtocolTest.groovy:

import org.apache.dubbo.remoting.http12.HttpUtils

import io.netty.handler.codec.http.multipart.DefaultHttpDataFactory

def "completed form request should release post data"() {
    given:
        HttpUtils.DATA_FACTORY.cleanAllHttpData()
        def request = new TestRequest(
            path: '/argTest',
            contentType: MediaType.APPLICATION_FROM_URLENCODED,
            body: 'name=Sam&age=8'
        )
    expect:
        runner.post(request) == 'Sam is 8 years old'
        trackedPostRequests() == 0
    cleanup:
        HttpUtils.DATA_FACTORY.cleanAllHttpData()
}

private static int trackedPostRequests() {
    def field = DefaultHttpDataFactory.getDeclaredField('requestFileDeleteMap')
    field.accessible = true
    return ((Map<?, ?>) field.get(HttpUtils.DATA_FACTORY)).size()
}

Run RestProtocolTest. The request succeeds and returns:

Sam is 8 years old

However, the assertion fails:

expected: 0
actual:   1

After running the same request 20 times, the number of tracked requests in the factory grows to 20 and remains unchanged after GC.

What you expected to happen

After the form request completes, its HttpPostRequestDecoder should be destroyed so that DefaultHttpDataFactory removes the associated request and HttpData. With no other concurrent form requests, the number of tracked requests should be 0.

The request currently completes successfully, but the factory still retains it and continues to grow as more form requests are processed.

Anything else

I also checked the related historical changes. PR #14741 changed the request-body buffer allocation path, while PR #14760 avoided creating a decoder for an empty request body. Neither change cleans up the decoder after a non-empty form request completes.

The relevant call chain is:

RestHttpMessageCodec.decode()
  → CompositeArgumentResolver
  → FallbackArgumentResolver
  → DefaultHttpRequest.parameter()
  → DefaultHttpRequest.getPostDecoder()
  → HttpUtils.createPostRequestDecoder()

HttpUtils creates the decoder with the static DATA_FACTORY and a newly created synthetic DefaultFullHttpRequest. Netty's factory stores the parsed form HttpData using that synthetic request as the key.

DefaultHttpDataFactory uses an identity-based map to strongly reference the synthetic request and its associated HttpData. The resource contract of HttpPostRequestDecoder explicitly requires calling the following method after the decoder is no longer needed:

postDecoder.destroy();

This removes the synthetic request entry from the factory and releases the associated data. The current production code does not destroy the decoder when request processing terminates, so these entries remain reachable through the static factory.

The default implementation of AbstractServerTransportListener.onDataFinally() only closes the current inbound message, while GenericHttp2ServerTransportListener overrides it with a no-op. Neither path releases the adapted DefaultHttpRequest or destroys its cached decoder. Completion of inbound message processing is not the same as completion of the overall request or response lifecycle.

Requests that do not trigger form or parameter parsing, and empty-body requests for which createPostRequestDecoder() returns null, do not create such entries.

Because DefaultHttpRequest lazily creates and caches the decoder, it needs an explicit cleanup mechanism that can be invoked from the terminal server request lifecycle. Cleanup should cover successful completion, failure, cancellation, and transport closure, and should run only after application code can no longer access request parameters or multipart parts.

Destroying the decoder immediately after argument resolution would be unsafe because multipart FileUpload objects returned by part() or parts() still depend on its underlying HttpData and may be consumed later by application code.

Do you have a (mini) reproduction demo?

  • Yes, I have a minimal reproduction demo to help resolve this issue more effectively!

Are you willing to submit a pull request to fix on your own?

  • Yes I am willing to submit a pull request on my own!

Code of Conduct

Activity

  1. added
    help wantedEverything needs help from contributors
    and removed on Aug 3, 2026
  2. soumadipcodeEnthusiast commented on Aug 6, 2026

    @soumadipcodeEnthusiast

    @juzi050 @014-code — since there are already two PRs up for this (#16411 and #16412), a few questions before I try anything myself, so I don't duplicate effort:

    Do either of the two PRs cover all four terminal paths called out in the issue — successful completion, failure, cancellation, and transport closure? The issue specifically notes onDataFinally() is a no-op in GenericHttp2ServerTransportListener, so I want to check cancellation/transport-closure cleanup isn't only handled on the happy path.
    How do the two approaches handle the multipart safety concern mentioned in the issue — destroying the decoder too early would break FileUpload objects returned by part()/parts() if application code hasn't consumed them yet. Do both PRs defer cleanup to the same terminal lifecycle hook, or do they differ here?
    Is one of the two approaches currently favored (more complete, better test coverage), or are they both still open for comparison?

    Since a fix is already in flight from two directions, would it be more useful for me to review/test #16411 and #16412 against the multipart + cancellation edge cases rather than write a third implementation? Happy to help either way.

  3. hhhhzzzj commented on Aug 31, 2026

    @hhhhzzzj

    I looked into the Triple REST request handling path in dubbo-rpc-triple.

    When handling form/multipart requests, HttpPostRequestDecoder is instantiated to parse request attributes, but its destroy() / cleanFiles() lifecycle hook is never triggered upon request completion or channel inactive events. This leaves decoder-allocated buffer chunks and temporary disk files uncollected.

    Adding a cleanup call in the REST handler completion filter and adding a unit test for multipart form request lifecycle should resolve this cleanly. If this approach works, I would be happy to submit a PR.

  4. LeonxLJX commented on Sep 1, 2026

    @LeonxLJX

    I'd like to take this one ([Bug] Triple REST does not clean up HttpPostRequestDecoder a). I'll look into the root cause and follow up with a PR. (claiming via @LeonxLJX)

  5. LeonxLJX commented on Sep 1, 2026

    @LeonxLJX

    I'd like to take this one ([Bug] Triple REST does not clean up HttpPostRequestDecoder a). I'll look into the root cause and follow up with a PR. (claiming via @LeonxLJX)

  6. LeonxLJX commented on Sep 1, 2026

    @LeonxLJX

    I'd like to take this one ([Bug] Triple REST does not clean up HttpPostRequestDecoder a). I'll dig into the root cause and follow up with a PR shortly. (claiming via @LeonxLJX)

  7. Asthenia0412 commented on Oct 13, 2026

    @Asthenia0412

    PR opened: fix(triple): prevent HttpPostRequestDecoder memory leak after form requests

    PR: #16504

    Summary of changes:

    • Added destroyPostDecoder() default method to HttpRequest interface
    • Added concrete implementation in DefaultHttpRequest that calls postDecoder.destroy()
    • Prevents DefaultHttpDataFactory from retaining request entries indefinitely
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedEverything needs help from contributors

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions