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

Concurrent AVP encoding issue - AvpDataException: Not enough data in buffer! #1

Closed
tomazas opened this issue Aug 17, 2015 · 1 comment
Assignees
Labels

Comments

@tomazas
Copy link

tomazas commented Aug 17, 2015

Hi all,

We have found 2 concurrency issues with AVP encoding/decoding when doing load tests.

  1. Ecoding problem causes such exceptions:
2015-08-14 13:47:23,832 DEBUG [org.jdiameter.client.impl.controller.PeerImpl] (TCPReader-1) internalError
org.jdiameter.client.api.io.TransportException: Avp Data Exception:
        at org.jdiameter.client.impl.transport.tcp.TCPClientConnection.onEvent(TCPClientConnection.java:319)
        at org.jdiameter.client.impl.transport.tcp.TCPClientConnection.onAvpDataException(TCPClientConnection.java:276)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.seekMessage(TCPTransportClient.java:446)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.append(TCPTransportClient.java:392)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.run(TCPTransportClient.java:213)
        at java.lang.Thread.run(Thread.java:745)
Caused by: org.jdiameter.api.AvpDataException: org.jdiameter.api.AvpDataException: Not enough data in buffer!
        at org.jdiameter.client.impl.parser.MessageParser.createMessage(MessageParser.java:121)
        at org.jdiameter.client.impl.transport.tcp.TCPClientConnection.onEvent(TCPClientConnection.java:314)
        at org.jdiameter.client.impl.transport.tcp.TCPClientConnection.onMessageReceived(TCPClientConnection.java:271)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.seekMessage(TCPTransportClient.java:440)
        ... 3 more
Caused by: org.jdiameter.api.AvpDataException: Not enough data in buffer!
        at org.jdiameter.client.impl.parser.ElementParser.decodeAvpSet(ElementParser.java:319)
        at org.jdiameter.client.impl.parser.MessageParser.createMessage(MessageParser.java:116)
        ... 6 more

And also a BufferOverflow in the end:

        2015-08-14 13:47:23,835 ERROR [org.jdiameter.client.impl.transport.tcp.TCPTransportClient] (TCPReader-1) Buffer overflow occured
java.nio.BufferOverflowException
        at java.nio.HeapByteBuffer.put(HeapByteBuffer.java:183)
        at java.nio.ByteBuffer.put(ByteBuffer.java:832)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.append(TCPTransportClient.java:385)
        at org.jdiameter.client.impl.transport.tcp.TCPTransportClient.run(TCPTransportClient.java:213)
        at java.lang.Thread.run(Thread.java:745)

Problem - encoded/decoded Diameter packets get corrupted (missing child AVPs).

Reason - This is due to the fact that the function getGrouped that converts AvpImpl.java rawData to groupedData is called concurrently when doing encodeAvpSet(). If this is done in the loop before data.write(raw) operation in encodeAvpSet, then all child AVPs could be left unencoded (not written) to the packet (since avp.getRaw().length becomes 0). And since a correct AVP length is written here, the packet gets corrupted.

2.We also obtained a NullpointerException in SessionImpl.java, when doing a release, due to the fact that container concurrently was set to null via setContainer, so this was also fixed. However, don't know what the "// FIXME" is for, but I guess it relates to the same problem.

With the #2 pull I propose a possible fix for this issue. Also to encapsulate more the existing data in AvpImpl.java, i.e. make rawData/groupedData not accessible from outside. ALSO to forbid to swap between rawData/groupedData in runtime, i.e. to not change created AVP.

AlerantAppNGIN referenced this issue in AlerantAppNGIN/jdiameter Sep 24, 2015
@brainslog
Copy link
Contributor

Thanks for reporting this and submitting the patch. I will be reviewing it shortly and applying it. Unfortunately the PR has been updated with other commits, would be good that it would be isolated.. the situation where the peer gets added to the peer table multiple times has been fixed with 547b3b1

@brainslog brainslog added the bug label Nov 24, 2015
@brainslog brainslog self-assigned this Nov 24, 2015
ammendonca added a commit that referenced this issue May 16, 2017
ammendonca added a commit that referenced this issue May 16, 2017
Also add length to Avp `toString` method.

Fixes #1, closes #2
elost pushed a commit to elost/jdiameter that referenced this issue Aug 5, 2019
…ack-start

peer failed to start on Stack.start because of race condition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants