Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APPLICATION_CLOSE event overrides PACKET_ARRIVAL #2

Open
btuan opened this issue Feb 13, 2016 · 0 comments
Open

APPLICATION_CLOSE event overrides PACKET_ARRIVAL #2

btuan opened this issue Feb 13, 2016 · 0 comments

Comments

@btuan
Copy link

btuan commented Feb 13, 2016

Handling of APPLICATION_CLOSE condvar flag broadcasts in tcp_thread.c would override PACKET_ARRIVAL events, necessitating clunky handling of packets in event of APPLICATION_CLOSE.

Scenario in which this would be a problem:

  • Both clients start in ESTABLISHED state
  • Client 1 calls APPLICATION_CLOSE, sends FIN packet, enters FIN_WAIT_1
  • Client 2 generates PACKET_ARRIVAL event
  • Client 2 calls APPLICATION_CLOSE event
  • At this point socket_state->flags.app_close == socket_state->flags.net_recv == 1
  • For Client 2, cascading if statements call handling of APPLICATION_CLOSE in state ESTABLISHED, then PACKET_ARRIVAL is later handled after Client 1 enters FIN_WAIT_1, violating TCP state rules

Proposed solutions:

  • Transpose the if clauses, so that flags.net_recv is considered first.
  • Add another conditional in the code, perhaps something like this:
if (socket_state->flags.app_close)
{
    int packet_arrival = 0;
    chilog(TRACE, "Event received: app_close");
    socket_state->flags.app_close = 0;
    if (socket_state->flags.net_recv)
    {
        packet_arrival = 1;
        chilog(TRACE, "Event received: net_recv");
        socket_state->flags.net_recv = 0;
    }
    pthread_mutex_unlock(&socket_state->lock_event);

    if (packet_arrival)
    {
        chitcpd_dispatch_tcp(si, entry, PACKET_ARRIVAL);
    }
    chitcpd_dispatch_tcp(si, entry, APPLICATION_CLOSE);
}

@borjasotomayor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant