If the provided context is canceled/expires after connReader and
connWriter are started, the resultant *Conn will be nil causing their
goroutines to be leaked.
* The check for the header size was preventing us from moving on if the body length was smaller than the standard frame header (8 bytes).
Moving it within the portion of the reader that deals with parsing headers.
* update changelog and some comments
---------
Co-authored-by: Richard Park <ripark@microsoft.com>
Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>
* Allow cross receiver message settlement
It's not prevented at present, but can cause a memory leak due to
entries never being removed from the unsettledMessages map.
When a received message isn't settled, associate its receiver with the
message. The settlement APIs will direct to the associated receiver.
* mark message as settled when RSM is mode first
set rcv to nil when message has been settled
* add some additional comments
* improve comment
* Roll back sender's delivery count and link credit
When a transfer fails to be sent due to context cancellation/timeout,
roll back the updates to delivery count and link credit.
* fix misspelling
* Fix potential hang in Sender.Send()
There were two observers of env.Sent, the session mux and the sender.
If the sender was the first to read from the channel, this would cause
the session mux to be blocked.
In order to support multiple observers, the channels must be treated as
semaphores which are signaled when closed.
Don't terminate a session's mux if a transfer wasn't sent due to
context cancellation/timeout (this is not terminal).
* simplify
* improve naming
* Fix non-determinism in fake NetConn.Write
Serialize the responses to write so that they show up in their specified
order. This was responsible for some spurious test failures.
Consolidate creation of senders and receivers with test hooks.
Make sending and receiving of frames log level 0 to get the minimum
amount of logging that can be useful.
* check for context cancelled
* add unit test for Conn.getWriteTimeout and update changelog
* use delta comparison
* log error verbatim
NetConn.Write will now send its payload to Conn on a separate goroutine,
allowing proper simulation of a delayed response.
A fake responder now returns a fake.Response that contains the payload
and any write delay.
Added newResponse() helper for returning frames without delay.
* Properly distinguish between input and output link handles
Per spec, peers can have different values for link handles. When
sending a frame, its Handle is set to the output (our) handle. When
receiving a frame, its Handle is set to the input (remote) handle.
Fixed session data structures to properly map an input handle to a link.
Renamed handle fields and supporting types to follow spec nomenclature.
Added a live test that reproduced the issue.
* set release date in changelog
* add debug logging message on link attach complete
* bump release date
* Make writes deterministic
Added Conn.WriteDeadline to control the write deadline when writing to
net.Conn. The default value is 30 seconds. This can be overridden by
APIs that take a context and the context has a deadline/timeout.
For APIs that directly send frames, wait for the frame to be written to
the underlying net.Conn before considering the write a success.
* add clarifying comments on Sent channel usage
* don't send if the deadline has exceeded
* don't allow cancellation of sending a frame
* remove TODO
* add TODO for receiver disposition ack timeout
* return transfer limit exceeded error for sender
* add another clarifying comment
* Don't leak message settlement on context cancel/timeout
For receivers in mode second, handle clean-up of in-flight messages from
the mux instead of the disposition call. This ensures that the link
credit is reclaimed if the disposition API is cancelled or times out
waiting for the disposition acknowledgement.
* fix semaphore release count
* Robust handling of context expiration/cancellation
If a context expires or is cancelled during session/link creation while
waiting for the ack, add the instance to a slice for later cleanup.
Removed force-closing of sessions/links.
Exhausting available session channels will return a *ConnError and close
the Conn.
Exhausting available link handles will return a *SessionError and close
the Session.
* add some additional tests
* add logging for abandoned sessions and links
* fix off-by-one error for default max links value
* Sending frames synchronizes with mux
Session mux no longer consumes frames once the end performative has been
sent. Instead, added channel endSent that links can select on.
Added method link.txFrame for proper muxing of frames to session.
Receivers send disposition frames through the mux to properly sync with
the receiver being closed.
Senders disable the outgoing transfers channel once close has started.
* minor tweaks
* fix comment
* don't send session flow frames after end performative
Include address of Conn/Session/Sender/Receiver when writing log
entries. This helps when a process contains multiple instances.
Include the link name in the error string when a link is forcibly
closed.
Include the channel number in the error string when a session is
forcibly closed.
Add log entries when session/link is forcibly closed.
Add log entry for muxing from Session to Conn.
Bumped up context timeout in TestFuzzConnCrashers. This is unrelated to
the race change. Some iterations of the test starting failing with a
context.DeadlineExceeded error.
* Honor context cancellation in Dial and NewConn
Previously they only honored a deadline, now they handle both.
TLS dialer now calls DialContext().
* make linter happy
* switch to context.WithCancel just because
If Close() exits due to cancellation/timeout and the mux is still
running, subsequent calls to Close() will read from doneErr which will
race with the mux.
Record closing state while synchronizing with the mux and cache the
value so subsequent calls return the same value.
* Clean up tests that trigger the race detector
Added some mux test hooks and supporting test infrastructure.
* fix enabling of race detection in CI
fixed race in logging
cleaned up a few logging statements for consistency
* Don't swallow unexpected frames
Terminate a session/link on receiving an unexpected frame.
The change uncovered a race introduced in #223 where it's possible to
send an *Error to a closed s.close causing a panic.
* cover a few more cases
* tweak test
* send detach performative on client-side close due to error
* include context channel when initiating shutdown
added some additional logging and added some missing : in log statements
* switch back to using close struct{} channel
* Clean up state on context timeout/cancellation
Document the potential for connection errors.
This required refactoring the link closing code. It now follows the
same pattern as Session.
* reword comment and ensure force-close only happens once
* Don't swallow unrecognized link handles
Per spec section 2.8.17, the session must be terminated.
* fix potential hang in session mux
* fix race in test
* add two more missed cases
* add logging and consolidate
* Disable Session sending frames after end performative
This is against the spec and results in the peer terminating the
connection. Just ignore any outgoing frames.
* add debug logging when discarding frames
* Replace messages channel with a queue (#244)
* Replace messages channel with a queue
The Receiver.messages channel placed an arbitrary restriction on the
amount of link credit that could be issued to a Receiver. Once the
Receiver has been created, it can never be issued credit that exceeds
the size of the channel.
In addition, if the Receiver were to receive messages that exceeds the
size of the channel, due to flow control bugs or other, the writes to
the channel could block leading to hangs.
The channel has been replaced with a segmented FIFO queue. While this
in theory could allow for unbounded growth, the reality is that the
total size can never be greater than that of the Session's incoming
window.
* use Ring from standard library
* switch to require.Same for pointer comparisons
fixed issue in last seg check and added another test
* refine and add another test
* size message queue to the incoming window
* add Holder[T] to abstract syncronized access
* Remove concept of "max credit" from Receivers (#251)
* Remove concept of "max credit" from Receivers
This exposed an implementation detail of how Receivers worked. With the
removal of the Message channel, this is no longer required and never
really made sense anyways.
The MaxCredit and ManualCredits options have been consolidated into a
single Credit option. This is used to set the total credit for a
Receiver in auto-flow mode, or to disable auto-flow.
Disposition batching needed some slight refactoring. Now you can
directly specify the batch size instead of it being tied to link credit.
Received messages that are sender-settled now correctly count towards a
Receiver's credit and must be settled to reclaim it.
* fix linter
* back out double-buffering change for now
* refine threshold logic
* switch to mutex instead of channel for better perf
* add back fix to remove double buffering
* rename local var for clarity
* revert cosmetic change
* Replace incoming frame channels with queues (#253)
* Replace incoming frame channels with queues
The rx channel in Session and link has been replaced with a queue. This
ensures that parents enqueueing frames to their children is always a non
blocking operation and removes the need to "pump the mux".
Consolidated some link creation code into newLink().
Links echoing a flow frame now correctly include session info.
Reverted workaround for sending disposition frames from senders as we
can now send directly from muxHandleFrame.
* minor cleanup from review
* Remove configuration of session window (#257)
The values weren't actually honored which is ok per spec, so remove them
from the public surface area.
Cleaned up handling of sender link credit. Since we don't use
availableCredit, it's been removed.
* Improve handling of sender-settled messages (#258)
Reclaim credit for sender-settled messages when Prefetch or Receive are
called. This removes the need to explicitly settle the message.