adamfisk / LittleProxy

High performance HTTP proxy originally written by your friends at Lantern and now maintained by a stellar group of volunteer open source programmers.
https://www.getlantern.org
Apache License 2.0
2.04k stars 775 forks source link

Error in connection counting #9

Closed neotyk closed 13 years ago

neotyk commented 13 years ago

In logs I found following:

37413336 [New I/O server worker #1-3] WARN org.littleshoot.proxy.HttpRequestHandler - Closing all proxy to web channels for this browser to proxy connection!!! 37413336 [New I/O client worker #1-3] WARN org.littleshoot.proxy.HttpRelayingHandler - Closing browser to proxy channel: [id: 0x0102401e, /172.16.10.35:50880 :> /172.16.10.50:9090] 37413336 [New I/O client worker #1-3] ERROR org.littleshoot.proxy.HttpRequestHandler - Something's amiss. We have -1 and 0 connections stored

What made me worried is last line: "We have -1 and 0 conn...". Apparently this should not happen. I've tried fixing it in our clone: https://github.com/sourcesense/LittleProxy Though we still see this error.

Please help in fixing it.

neotyk commented 13 years ago

I managed to reproduce it in local environment.

Setup is as follows: Nginx serving big files. Asynchronous HTTP Client talking to Nginx via proxy, requesting 10 files in paralel.

Result: "Something's amiss" in logs, and not all content received.

adamfisk commented 13 years ago

Many apologies for the delay addressing this. I've seen this a few times in the logs as well. I think it might be since I updated to the latest Netty. I'm getting pulled in a million different directions right now, but you might try reverting to the prior Netty and seeing if that fixes it.

You're able to consistently reproduce it, and it definitely causes errors? I haven't actually seen that issue cause errors directly, but I've been focused on things built on LittleProxy and not LittleProxy itself for a little while now.

zdila commented 13 years ago

Affects me too.

adamfisk commented 13 years ago

Are you guys using 0.3 or the 0.4-SNAPSHOT? I apologize for neglecting LittleProxy these last few months -- super tied up with Lantern, an upcoming censorship circumvention tool that also uses LittleProxy.

I have not been seeing this issue, but I've also started deploying snapshots to nexus. Try this maven config:

    <dependency>
        <groupId>org.littleshoot</groupId>
        <artifactId>littleproxy</artifactId>
        <version>0.4-SNAPSHOT</version>
    </dependency>

    <repository>
        <id>sonatype-nexus-snapshots</id>
        <name>Sonatype Nexus Snapshots</name>
        <url>https://oss.sonatype.org/content/repositories/snapshots</url>
        <releases>
            <enabled>false</enabled>
        </releases>
        <snapshots>
            <enabled>true</enabled>
        </snapshots>
    </repository>
zdila commented 13 years ago

I am using 0.4-SNAPSHOT with your maven config and I am seeing this error when visiting for example http://www.sme.sk/.

zdila commented 13 years ago

I probably found the issue. In the HttpRequestHandler.channelClosed the call future.getChannel().close() causes call to HttpRequestHandler.onRelayChannelClose. In HttpRequestHandler.channelClosed the numWebConnections is zeroed but HttpRequestHandler.onRelayChannelClose is called for every closed channel again and thus numWebConnections is decreased (often below zero). Another sympthom is that endpointsToChannelFutures.remove(key) returns null in HttpRequestHandler.onRelayChannelClose.

Solution is to delete following lines from HttpRequestHandler.channelClosed: endpointsToChannelFutures.clear(); numWebConnections = 0;

I am beginner with netty so I may be wrong ;-).

adamfisk commented 13 years ago

Another great catch Martin -- thanks. I just committed and deployed this fix as well. Can you guys verify that you no longer see the issue?

adamfisk commented 13 years ago

I should also note I don't think this should have any functional effect. I only added that code to make sure the logic was generally correct and that connections weren't getting out of sync, but the tracking logic itself was wrong.

Thanks again Martin. I'm stretched thin on my end, so this is really a big help.

adamfisk commented 13 years ago

Closing this as it appears to be fixed on master.