Smack

Timing of the Keep Alive Thread Could Break TLS

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.0.0, 2.1.0, 2.2.0
  • Fix Version/s: 2.2.1
  • Component/s: Core
  • Labels:
    None
  • Acceptance Test - Add?:
    No

Description

A period exception has been occuring where TLS would mysteriously fail. Syncornization between the main thread and the PacketReader was originally thought to be the culprit but thanks to Skip Dooley's dedication to the problem it was found to be an issue with the Keep Alive thread spinning up before TLS could successfully complete.

Issue Links

is related to

Bug - A problem which impairs or prevents the functions of the product. SMACK-140 Connection Negotiation Done Asynchronously

  • Critical - Crashes, loss of data, severe memory leak.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Gaston Dombiak added a comment -

Implemented fixes:

1) Start keep alive thread after TLS (if available) was negotiated or if server is not xmpp 1.0 compliant then after stream header was received

2) Keep alive thread will initially wait for 15 seconds before starting to work

3) Keep alive thread will send a hearbeat every 30 seconds (by default) since the last packet was sent to the server. In other words, connections that are sending stanzas all the time will no need to send heartbeats to keep the connection alive.

Show
Gaston Dombiak added a comment - Implemented fixes: 1) Start keep alive thread after TLS (if available) was negotiated or if server is not xmpp 1.0 compliant then after stream header was received 2) Keep alive thread will initially wait for 15 seconds before starting to work 3) Keep alive thread will send a hearbeat every 30 seconds (by default) since the last packet was sent to the server. In other words, connections that are sending stanzas all the time will no need to send heartbeats to keep the connection alive.
Hide
Permalink
Srgjan Srepfler added a comment -

I don't think this bug should be closed, we're still seeing problems (while using it via mule).

Show
Srgjan Srepfler added a comment - I don't think this bug should be closed, we're still seeing problems (while using it via mule).
Hide
Permalink
Guillaume Perrot added a comment -

Waiting 15s is not a solution, it should wait for an exact condition...
I also fixed a similar big race condition when enabling the compression.
The code never waits for the stream to be restarted, for example the binding IQ can be sent before the server sent us the new stream...
Please stop waiting arbitrary timeouts and use Condition objects (or wait/notify).

Show
Guillaume Perrot added a comment - Waiting 15s is not a solution, it should wait for an exact condition... I also fixed a similar big race condition when enabling the compression. The code never waits for the stream to be restarted, for example the binding IQ can be sent before the server sent us the new stream... Please stop waiting arbitrary timeouts and use Condition objects (or wait/notify).

People

  • Assignee:
    Gaston Dombiak
    Reporter:
    Alex Wenckus
Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: