onCompleteFailure called multiple times #9476

forfreeday · 2023-03-08T07:23:30Z

Jetty version(s)
Jetty 9.4.51

Java version/vendor (use: java -version)
java version 1.8.0_161

OS type/version
CentOS

Description

When I upgrade to Jetty 9.4.51, this exception will appear in the log every day.
What caused this anomaly?

04:04:21.744 WARN  [qtp903644315-163107] [o.e.j.i.AbstractConnection](AbstractConnection.java:96)
java.lang.NullPointerException: null
        at org.eclipse.jetty.io.AbstractConnection.lambda$failedCallback$0(AbstractConnection.java:92)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
        at java.lang.Thread.run(Thread.java:748)

The text was updated successfully, but these errors were encountered:

joakime · 2023-03-08T12:25:15Z

Jetty 9.x is now at End of Community Support.

See

End of Community Support for Jetty 9.x - June 2022 #7958

You should be upgrading to Jetty 10 or Jetty 11 at this point.

forfreeday · 2023-03-15T03:22:00Z

Our system is JDK8, can not upgrade to a higher version, can only use jetty9

olamy · 2023-03-15T03:36:47Z

Our system is JDK8, can not upgrade to a higher version, can only use jetty9

You should definitely read #7958 and how to get Jetty 9.x support.

JoergHeinicke5005 · 2023-04-02T20:18:45Z

I do understand that you can't support multiple major versions in parallel, but you have to end support at some point. Where I'm not following, is, that a NullPointerException had recently been introduced (with v9.4.51, which means it got broken by an actively performed change, i.e. while in EOL) and nonetheless you don't want to take care of it.
If I understand correctly, this new patch version v9.4.51 is for professional support of Jetty. So, there is the slight chance that the NPE gets silently fixed the same way it got introduced. Then again, it would be easy to provide tracking of such an issue in a public ticket like this one.
Btw., if running Spring Boot 2 with Jetty, v9 is still the default version (even though it can be switched to v10). I doubt these will be the last occurrences you'll see this issue being reported.

schrotti74 · 2023-05-25T12:24:39Z

I agree. Also recently upgraded to 9.4.51 in pre-prod (e.g. loadtest) where we have seen the NPE. If this is some sort of regression recently introduced, it should be fixed in another 9.4.x based release.

gregw · 2023-05-25T13:33:54Z

Since we must have introduced in recent release, we will look at fixing it as well.

AN3Orik · 2023-06-05T16:12:48Z

Any news?

joakime · 2023-06-05T17:09:28Z

@AN3Orik since Jetty 9.x is End of Community Support, this is extremely low priority.

Our overwhelming focus is on Jetty 12, with support for Jetty 10 and Jetty 11 right now.

joakime · 2023-06-05T17:12:19Z

This is reported against AbstractConnection which has had no code changes since Aug 22, 2018

https://github.com/eclipse/jetty.project/commits/jetty-9.4.x/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java

Which was around the release of Jetty 9.4.12.

AN3Orik · 2023-06-05T17:36:09Z

It's start happening after changes in this commit i think
add some fixes to SizeLimitHandler

Failed callback added and this callback is null when AbstractConnection.failedCallback() method reached

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts · 2023-06-13T04:29:41Z

@AN3Orik SizeLimitHandler was only added in 9.4.50 and is not added to the server by default.

You must manually add it for it to ever be used, so unless you have started using this handler in 9.4.50 it is not related.

lachlan-roberts · 2023-06-13T05:00:39Z

I reviewed the 9.4.50 -> 9.4.51 diff and this almost certainly comes from this change #9062.

The failedCallback() method is only called from onCompleteFailure, and #9062 changes how this can be called.
@gregw @sbordet

Added a null check for the Callback parameter in o.e.j.iAbstractConnection.failedCallback(Callback, Throwable) so that the subsequent code-flow is not aborted; while the root cause may be ascertained in case debug logging is enabled. Further, in the case of a RejectedExecutionException when submitting the failCallback, the Runnable is run instead of directly invoking the Callback, so that exceptions in the Callback do not abort the synchronous code-flow.

sid-evolphin · 2023-06-20T08:22:27Z

Since we must have introduced in recent release, we will look at fixing it as well.

@gregw Please review the PR #9932 I have submitted to avoid a null Callback from derailing the synchronous flow subsequent to the invocation of o.e.j.i.AbstractConnection.failedCallback(Callback, Throwable). Also if debug logging is enabled, then the root cause may be ascertained for the NPE.

Thank you very much.

…lback 1. Fixed the usages of `Callback` returned by `o.e.j.s.HttpConnection.SendCallback.release()` to check for `null` before invoking the `failedCallback(Callback, Throwable)` or `succeeded()` methods. 2. Also, added a try-finally block around the Callback invocations in order to ensure that shutdownOutput will get performed irrespective of whether they end up throwing any exception.

@gregw

…w feedback Partially reverted `o.e.j.i.AbstractConnection.failedCallback(Callback, Throwable)` to not have a null check for the `Callback` parameter, based on review feedback from @gregw. This avoids changing the contract of the method parameter (from `@NonNull` to `@Nullable`).

gregw · 2023-06-20T12:30:25Z

@sbordet @lachlan-roberts I think this is actually a real bug in IteratingCallback in all our releases. I think we are able to call onCompleteFailure multiple times for the same failure.

gregw · 2023-06-20T12:44:47Z

@sbordet @lachlan-roberts @lorban The following "test" is passing when it should not. It shows that we can call onCompleteFailure multiple times. We can also call process after a failure!

    @Test
    public void testMultipleFailures() throws Exception
    {
        AtomicInteger process = new AtomicInteger();
        AtomicInteger failure = new AtomicInteger();
        IteratingCallback icb = new IteratingCallback()
        {
            @Override
            protected Action process() throws Throwable
            {
                process.incrementAndGet();
                return Action.SCHEDULED;
            }

            @Override
            protected void onCompleteFailure(Throwable cause)
            {
                super.onCompleteFailure(cause);
                failure.incrementAndGet();
            }
        };

        icb.iterate();
        assertEquals(1, process.get());
        assertEquals(0, failure.get());

        icb.failed(new Throwable("test1"));

        assertEquals(1, process.get());
        assertEquals(1, failure.get());

        icb.succeeded();
        assertEquals(2, process.get());
        assertEquals(1, failure.get());

        icb.failed(new Throwable("test1"));
        assertEquals(2, process.get());
        assertEquals(2, failure.get());

Change state to FAILED from PENDING state

sid-evolphin · 2023-06-20T14:32:29Z

Thanks a lot @gregw!

Change state to FAILED from PENDING state

gregw · 2023-06-21T09:21:34Z

merged through to 12.0.x

@forfreeday We are 99% sure this will fix your problem, but we did not have an exact reproducer of your exact case. Any chance you can test this out before we do a release?

markkolich · 2023-08-08T06:02:46Z

@gregw are you still looking for someone to try this out pre-release?

I can reproduce this NPE on 9.4.51.v20230217 pretty much at will, so I'm happy to try a snapshot with a fix and see if that resolves the bug.

If you need a guinea pig let me know what 9.x snapshot I should try.

gregw · 2023-08-08T08:24:52Z

@markkolich that would be good. I think the most recent nightly is https://oss.sonatype.org/service/local/repo_groups/jetty-with-staging/content/org/eclipse/jetty/jetty-distribution/9.4.52-SNAPSHOT/jetty-distribution-9.4.52-20230807.110254-149.tar.gz else you can build it from source.

forfreeday added the Bug For general bugs on Jetty side label Mar 8, 2023

joakime added the End-of-Life release label Mar 8, 2023

joakime closed this as completed Mar 15, 2023

joakime mentioned this issue Mar 21, 2023

NullPointerException in AbstractConnection #9522

Closed

gregw reopened this May 25, 2023

gregw self-assigned this May 25, 2023

joakime added the Low Priority label Jun 5, 2023

AN3Orik referenced this issue Jun 9, 2023

add some fixes to SizeLimitHandler

ec197a2

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw linked a pull request Jun 20, 2023 that will close this issue

Issue #9476 - Fix 9.4.51 NullPointerException #9932

Closed

gregw mentioned this issue Jun 20, 2023

Issue #9476 - Fix 9.4.51 NullPointerException #9932

Closed

gregw removed the Low Priority label Jun 20, 2023

gregw added a commit that referenced this issue Jun 20, 2023

Fix #9476 IteratingCallback multiple onCompleteFailure calls

557168d

Change state to FAILED from PENDING state

gregw mentioned this issue Jun 20, 2023

Fix #9476 IteratingCallback multiple onCompleteFailure calls #9935

Merged

gregw linked a pull request Jun 20, 2023 that will close this issue

Fix #9476 IteratingCallback multiple onCompleteFailure calls #9935

Merged

gregw changed the title ~~Jetty 9.4.51 NullPointerException~~ Jetty 9.4.51 onCompleteFailure called multiple times Jun 20, 2023

gregw added a commit that referenced this issue Jun 21, 2023

Fix #9476 IteratingCallback multiple onCompleteFailure calls (#9935)

d935651

Change state to FAILED from PENDING state

gregw added a commit that referenced this issue Jun 21, 2023

Fix #9476 IteratingCallback multiple onCompleteFailure calls

f3248de

Change state to FAILED from PENDING state

gregw mentioned this issue Jun 21, 2023

Fix #9476 IteratingCallback multiple onCompleteFailure calls #9940

Merged

gregw closed this as completed in 9041527 Jun 21, 2023

joakime changed the title ~~Jetty 9.4.51 onCompleteFailure called multiple times~~ onCompleteFailure called multiple times Aug 25, 2023

rymsha mentioned this issue Sep 8, 2023

Cannot invoke "org.eclipse.jetty.util.Callback.failed(java.lang.Throwable)" enonic/xp#10261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

onCompleteFailure called multiple times #9476

onCompleteFailure called multiple times #9476

forfreeday commented Mar 8, 2023

joakime commented Mar 8, 2023 •

edited

forfreeday commented Mar 15, 2023

olamy commented Mar 15, 2023

JoergHeinicke5005 commented Apr 2, 2023

schrotti74 commented May 25, 2023

gregw commented May 25, 2023

AN3Orik commented Jun 5, 2023

joakime commented Jun 5, 2023

joakime commented Jun 5, 2023

AN3Orik commented Jun 5, 2023

lachlan-roberts commented Jun 13, 2023

lachlan-roberts commented Jun 13, 2023

sid-evolphin commented Jun 20, 2023

gregw commented Jun 20, 2023

gregw commented Jun 20, 2023

sid-evolphin commented Jun 20, 2023

gregw commented Jun 21, 2023

markkolich commented Aug 8, 2023

gregw commented Aug 8, 2023

onCompleteFailure called multiple times #9476

onCompleteFailure called multiple times #9476

Comments

forfreeday commented Mar 8, 2023

joakime commented Mar 8, 2023 • edited

forfreeday commented Mar 15, 2023

olamy commented Mar 15, 2023

JoergHeinicke5005 commented Apr 2, 2023

schrotti74 commented May 25, 2023

gregw commented May 25, 2023

AN3Orik commented Jun 5, 2023

joakime commented Jun 5, 2023

joakime commented Jun 5, 2023

AN3Orik commented Jun 5, 2023

lachlan-roberts commented Jun 13, 2023

lachlan-roberts commented Jun 13, 2023

sid-evolphin commented Jun 20, 2023

gregw commented Jun 20, 2023

gregw commented Jun 20, 2023

sid-evolphin commented Jun 20, 2023

gregw commented Jun 21, 2023

markkolich commented Aug 8, 2023

gregw commented Aug 8, 2023

joakime commented Mar 8, 2023 •

edited