vp340 commented on code in PR #3521:
URL: https://github.com/apache/cxf/pull/3521#discussion_r4179190772
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingOutInterceptor.java:
##########
Review Comment:
@reta I completely agree.
[First scenario - Close twice]
I will try to reproduce the issue, but being so difficult I won't assure
anything (right now with work and university I'm too busy and lately I already
spend more time that I could afford on this issue ).
Today I also focus on the point:
`sender.send(event);
try {
// empty out the cache
cos.lockOutputStream();
cos.resetOut(null, false);
} catch (Exception ex) {
// ignore
}`
We know for sure that sender.send was executed (first log) ...
cos.lockOutputStream() should do nothing because outputLocked is already true
and It would provoke an infinite loop if it wasn't (I wonder why it's there at
this point) .... and as U say cos.resetOut(...) should unregister from the
queue. Maybe It's that exception ignored that is hiding something, but in my
custom implementation I put a warn log there but seen nothing in the log file
...
I'm start wondering with what version I was 'playing' that day. Because when
I discover my production was filled with tmp file I tried updating to 4.0.6
locally to fix that (I know we have old version, don't blame me :/ ) ... and
my local build stayed with that version for a while before updating to 4.0.11
so now I'm unsure which one I had that day .
Today I also found https://issues.apache.org/jira/browse/CXF-9110 ... that
in 4.0.7 fixed a memory leak similar to the one I found.
I think more and more that the two could be the same issue, even if I didn't
find a reference to double logging.
When I have time I will reput the 4.0.6 and try to see if there was a twice
logging there.
---
[Second scenario - Not close]
(for this I'm sure I reproduce the issue in 4.0.11)
As I wrote in the previous message I found that when java.io.IOException:
Connection reset by peer happens, the CachedOutputStream seems to be never
close.
Here I can simply reproduce the issue (just kill the connection while
writing RESP_OUT) ...
I don't know what approach do U prefer here..
`at
org.apache.catalina.connector.CoyoteOutputStream.write(CoyoteOutputStream.java:102)
at
org.apache.cxf.io.AbstractWrappedOutputStream.write(AbstractWrappedOutputStream.java:51)
at
org.apache.cxf.io.CacheAndWriteOutputStream.write(CacheAndWriteOutputStream.java:81)
at
org.apache.cxf.io.AbstractWrappedOutputStream.write(AbstractWrappedOutputStream.java:51)
at com.ctc.wstx.io.UTF8Writer.write(UTF8Writer.java:143)`
Being a client controlled bug I suppose we depend on the implementation (for
example in this scenario of tomcat). But I don't think that someone had to call
close for us.
In this case how we move?
Today just to try I made a cleanup interceptor that rewinding the cxf chain
check if the Exception was that and try to close the cos for us. It worked...
It log and unregister from the queue as excpected.
Let me know if U want to see it... I just need to prettify a little bit the
code to make it readable and push it.
It could be a good workaround if we can't control elsewhere.
Otherwise we need to act somewhere in AbstractWrappedOutputStream? and close
there if we catch that exception?
Thanks a lot for your time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
