Project

General

Profile

Bug #54

Multiple multi-reads (of size 1, typically) do not return the correct number of events.

Added by Nitesh Mor almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
libgdp
Start date:
09/23/2016
Due date:
% Done:

0%


Description

For a multi-read of size 1, a client should receive 2 events (GDP_EVENT_DATA followed by GDP_EVENT_EOS). In case of N multi-reads of size 1, this translates to 2*N events. But this doesn't always happen. In as small as 2 multi-reads, the number of actual events range from 2 to 4 across different runs.

Attached is a sample Python program to reproduce it; it attempts to implement an asynchronous read by calling multi-reads of size 1. The same behavior is seen in C as well.

The server returns the correct number of events (verified with WireShark). The bug is in the client library processing. My hunch is the bug lies is in the processing of the PDU starting at source:gdp/gdp_main.c#L425.

In source:gdp/gdp_priv.h#L409, the assumption is that a request can not have a WAITING status, while maintaining an active subscription (or multi-read, for that matter). The logic in source:gdp/gdp_main.c#L425 follows this assumption. However with quite a few multi-reads of size 1, this assumption fails: a reader may be WAITING on _gdp_invoke of a new multi-read command when a PDU as a result of a previous multi-read arrives.

Attached is a debug log for 2 multi-reads of size 1, which should see 4 events but only 2 events are passed back to the client. See line#150, where req->state is WAITING; the PDU that was received is the data from result of multi-read and the control flow goes to the if block at source:gdp/gdp_main.c#L425 rather than to source:gdp/gdp_main.c#L443

reader_test_async.py Magnifier (1.4 KB) Nitesh Mor, 09/23/2016 07:29 PM

multi-read.debug.log Magnifier (19.7 KB) Nitesh Mor, 09/23/2016 07:30 PM

History

#1 Updated by Nitesh Mor almost 4 years ago

commit:4d6ff457 seems to have it fixed. I can no longer reproduce the bug.

Leaving this open though, just in case you have more sub-issues to be addressed that I might not be aware of. Feel free to close it.

#2 Updated by Eric Allman almost 4 years ago

  • Status changed from New to Feedback

Should have been fixed in commit:4d6ff457. However, it turns out that small multi-reads still serialize behind the actual read command, so commit:8204d4b2 adds gdp_gcl_read_async, which asynchronously returns exactly one event PDU for each read.

#3 Updated by Nitesh Mor almost 4 years ago

  • Status changed from Feedback to Closed

Verified read_async works. Closing this. Thanks

Also available in: Atom PDF