Skip to content

Read timeouts in fsmpico #4

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.

Sign up for GitHub

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

Open
saebischer wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Read timeouts in fsmpico #4

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7fae5d
Read timeouts in fsmpico
Jan 29, 2018
3d290ab
Reply to ServiceReauth immediately
Feb 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 35 additions & 18 deletions src/fsmpico.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@

// Defines

#define READ_TIMEOUT (5000)
#define CONTINUOUS_TIMEOUT_LEEWAY (1000)
Copy link
Member

@dettonijr dettonijr Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we unify those two constants in a single leeway? They seem to have similar semantics.

#define RECONNECT_DELAY (10000)

// Structure definitions
Expand Down Expand Up @@ -111,6 +113,8 @@ struct _FsmPico {
FSMPICOSTATE state;
AuthFsmComms * comms;
void * user_data;

int reauthDelay;
};

// Function prototypes
Expand Down Expand Up @@ -165,6 +169,8 @@ FsmPico * fsmpico_new() {
fsmpico->comms->sessionEnded = FsmSessionEndedNull;
fsmpico->comms->statusUpdate = FsmStatusUpdateNull;

fsmpico->reauthDelay = 0;

return fsmpico;
}

Expand Down Expand Up @@ -348,12 +354,14 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) {
int timeout;
Buffer * message;
Buffer * dataread;
Buffer * extraDataToSend;
char status;

receivedExtraData = buffer_new(0);
message = buffer_new(0);
dataread = buffer_new(length);
buffer_append(dataread, data, length);
extraDataToSend = buffer_new(0);

// TODO: If the reads fail, should move to an error state
switch (fsmpico->state) {
Expand All @@ -364,6 +372,8 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) {
createMessagePicoAuth(fsmpico, message, fsmpico->extraData);
fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data);
stateTransition(fsmpico, FSMPICOSTATE_STATUS);
// set a timeout for waiting for the status message
fsmpico->comms->setTimeout(READ_TIMEOUT, fsmpico->user_data);
Copy link
Member

@llewelld llewelld Feb 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the pattern of fsmservice, the timeout on connect is sufficient. This timeout should be removed, unless there's a good reason for it that I'm missing?

}
break;
case FSMPICOSTATE_STATUS:
Expand All @@ -390,9 +400,13 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) {
result = readMessageServiceReauth(fsmpico, dataread, &timeout);
if (result) {
stateTransition(fsmpico, FSMPICOSTATE_PICOREAUTH);
LOG(LOG_DEBUG, "Timeout set to: %d", timeout);
// Wait for timeout
fsmpico->comms->setTimeout(timeout, fsmpico->user_data);
fsmpico->reauthDelay = timeout;
// reply with the PicoReauth message
createMessagePicoReauth(fsmpico, message, extraDataToSend);
fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data);
stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH);
// set a timeout for awaiting a response
fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data);
}
break;
default:
Expand All @@ -404,6 +418,7 @@ void fsmpico_read(FsmPico * fsmpico, char const * data, size_t length) {
buffer_delete(receivedExtraData);
buffer_delete(message);
buffer_delete(dataread);
buffer_delete(extraDataToSend);
}

/**
Expand All @@ -425,6 +440,8 @@ void fsmpico_connected(FsmPico * fsmpico) {
createMessageStart(fsmpico, message);
fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data);
stateTransition(fsmpico, FSMPICOSTATE_SERVICEAUTH);
// set a timeout for waiting for the service auth message
fsmpico->comms->setTimeout(READ_TIMEOUT, fsmpico->user_data);
break;
case FSMPICOSTATE_CONTSTARTPICO:
fsmpico->currentState = REAUTHSTATE_CONTINUE;
Expand Down Expand Up @@ -480,31 +497,31 @@ void fsmpico_disconnected(FsmPico * fsmpico) {
* @param fsmpico The object to apply to.
*/
void fsmpico_timeout(FsmPico * fsmpico) {
Buffer * extraData;
Buffer * message;

LOG(LOG_DEBUG, "Timeout");

extraData = buffer_new(0);
message = buffer_new(0);

switch (fsmpico->state) {
case FSMPICOSTATE_CONTSTARTPICO:
case FSMPICOSTATE_CONTSTARTPICO:
LOG(LOG_DEBUG, "Reconnecting for continuous authentication");
fsmpico->comms->reconnect(fsmpico->user_data);
break;
case FSMPICOSTATE_PICOREAUTH:
createMessagePicoReauth(fsmpico, message, extraData);
fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data);
stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH);
case FSMPICOSTATE_SERVICEAUTH:
case FSMPICOSTATE_STATUS:
case FSMPICOSTATE_SERVICEREAUTH:
// gave up waiting for a reply to a sent message, enter error state
stateTransition(fsmpico, FSMPICOSTATE_ERROR);
fsmpico->comms->error(fsmpico->user_data);
break;
default:
case FSMPICOSTATE_INVALID:
case FSMPICOSTATE_ERROR:
// in these states, the state machine is stopped, so ignore it (but log)
LOG(LOG_DEBUG, "Timer fired during an invalid state");
break;
default:
// in any other state, we're not expecting a timeout and this is an error
stateTransition(fsmpico, FSMPICOSTATE_ERROR);
fsmpico->comms->error(fsmpico->user_data);
break;
}

buffer_delete(extraData);
buffer_delete(message);
}


Expand Down