The request never returns

[paopaol]

When a request is sent and a response is waiting, another thread calls the delete route. Then the request will be blocked forever. After analyzing the code, it seems that the Notify of the response will never be called because the Recv method of AmsConnection will throw an exception.

void AmsConnection::Recv()
{
    AmsTcpHeader amsTcpHeader;
    AoEHeader aoeHeader;
    for ( ; ownIp; ) {
       // Receive maybe throw expection.when socket shutdown called.
       //then the response Notify never called
        Receive(amsTcpHeader);
        if (amsTcpHeader.length() < sizeof(aoeHeader)) {
            LOG_WARN("Frame to short to be AoE");
            ReceiveJunk(amsTcpHeader.length());
            continue;
        }

        Receive(aoeHeader);
        if (aoeHeader.cmdId() == AoEHeader::DEVICE_NOTIFICATION) {
            ReceiveNotification(aoeHeader);
            continue;
        }

        auto response = GetPending(aoeHeader.invokeId(), aoeHeader.targetPort());
        if (!response) {
            LOG_WARN("No response pending");
            ReceiveJunk(aoeHeader.length());
            continue;
        }

        switch (aoeHeader.cmdId()) {
        case AoEHeader::READ_DEVICE_INFO:
        case AoEHeader::WRITE:
        case AoEHeader::READ_STATE:
        case AoEHeader::WRITE_CONTROL:
        case AoEHeader::ADD_DEVICE_NOTIFICATION:
        case AoEHeader::DEL_DEVICE_NOTIFICATION:
            ReceiveFrame<AoEResponseHeader>(response, aoeHeader.length(), aoeHeader.errorCode());
            continue;

[pbruenn]

Where exactly? Until GetPending() the response is not marked as in use so the request will timeout. And after GetPending() I see no code path without Notify.

[paopaol]

Where exactly? Until GetPending() the response is not marked as in use so the request will timeout. And after GetPending() I see no code path without Notify.

What does it mean that the response is not marked as in use? I checked the code and found that when the data is sent out, the response will wait on a condition variable. But if Receive(amsTcpHeader) throws an exception, then GetPending will never be called. Do you mean that if Receive(amsTcpHeader) throws an exception, the condition variable will also time out and be awakened by itself?

[pbruenn]

I repeat "Where exactly?".

Here we wait with a timeout:

ADS/AdsLib/standalone/AmsConnection.cpp

Line 25 in 20d4a67

cv.wait_until(lock, request.load()->deadline, [&]() { return !invokeId.load(); });

Only after invokeId wasn't consumed by GetPending

ADS/AdsLib/standalone/AmsConnection.cpp

Line 169 in 20d4a67

if (queue[portIndex].invokeId.compare_exchange_strong(currentId, 0)) {

The Wait() will return

ADS/AdsLib/standalone/AmsConnection.cpp

Line 29 in 20d4a67

return ADSERR_CLIENT_SYNCTIMEOUT;

Only after invokeId was consumed we would wait a second time without timeout

ADS/AdsLib/standalone/AmsConnection.cpp

Line 33 in 20d4a67

cv.wait(lock, [&]() { return errorCode != WAITING_FOR_RESPONSE; });

But in these cases GetPending() was successful and we either got an unknown AMS command id

ADS/AdsLib/standalone/AmsConnection.cpp

Line 350 in 20d4a67

response->Notify(ADSERR_CLIENT_SYNCRESINVALID);

Or we call ReceiveFrame()

ADS/AdsLib/standalone/AmsConnection.cpp

Line 340 in 20d4a67

ReceiveFrame<AoEResponseHeader>(response, aoeHeader.length(), aoeHeader.errorCode());

ADS/AdsLib/standalone/AmsConnection.cpp

Line 345 in 20d4a67

ReceiveFrame<AoEReadResponseHeader>(response, aoeHeader.length(), aoeHeader.errorCode());

And I see no codepath in ReceiveFrame without a Notify().