Skip to content

Conversation

@bhaan
Copy link
Contributor

@bhaan bhaan commented Jul 11, 2016

The current session event loop has a primitive approach toward handling outgoing messages sent from application code. A call to quickfix.SendToTarget essentially adds the message to a queue, which the event loop picks up during it's next pass. This is limiting for applications that want to account for a message send failure, as there is no way for the event loop to communicate that back to application code.

These commits propose the following changes:

  • Remove the toSend message queue
  • Add a dedicated channel in the event loop for sending messages.
  • Have SendToTarget push messages to the event loop, and block on a corresponding error channel
  • Force the FromAdmin/App callback to occur on a dedicated goroutine. This creates separation between "application space" and "session space", and is required to prevent deadlocks on calls to SendToTarget during the FromAdmin/App callback.
  • Separate session state message handling into three parts: 1) Vaildation 2) FromAdmin/App callback 3) Processing for admin and rejected messages. This is required to enable the above changes.

s.toSend = make(chan sendRequest)
s.resendIn = make(chan Message, 1)

type fromCallback struct {
Copy link

Choose a reason for hiding this comment

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

any ideas on what the compiler will make of these stucts / anonymous functions getting continuously redefined from within run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the size of golang structs I believe are just 8 bytes in addition to the types they enclose. So every time you'd call run, you'd be allocating, on the stack, 8 bytes + size of Message + size of MessageRejectError. Which is negligible considering when and how many times run is expected to be called.

@cbusbey cbusbey added the Bug Behavior not matching expected label Jul 11, 2016
@cbusbey cbusbey added this to the v0.4.0 milestone Jul 11, 2016
@cbusbey cbusbey merged commit d37efec into quickfixgo:master Jul 11, 2016
@cbusbey
Copy link
Contributor

cbusbey commented Jul 11, 2016

🍬 🌵

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Behavior not matching expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants