Keep a local (internal) shared_ptr whenever
user callbacks are run. This allows user
callbacks to drop (reset()/dtor) Operations
or Monitors safely.
Ref must to captured before mutex is locked,
which is safe as the internal weak_ptr is
not changed after build() (despite being non-const)
Thus far this has been dead code as ServerChannel::getRequest()
returned a Destroyable which was actually a BaseChannelRequester
sub-class, not a ChannelRequest sub-class.
Implement flow control windows in the server itself,
so that this applies to all Monitor.
Then the reportRemoteQueueStatus() callback is only needed
to deliver the initial window update.
This is done by not calling Monitor::release() immediately
when pipeline=true. Rather, hold onto MonitorElement s
until an ack is received.
Also addresses a race with clearing _channelMonitor
vs. having send() queued at/after destroy().
There is a lock order violation when a server force disconnects it's clients while sending a final message to it (or concurrent close, or probably others). The entry points are channelStateChange() and operation send().
Fix is not to hold the channel mutex when destroy()ing operations.
epics::pvAccess::ServerChannelRPCRequesterImpl::destroy
locks RPC requester mutex
epics::pvAccess::ServerChannel::destroy
locks channel mutex
epics::pvAccess::ServerChannelRequesterImpl::channelStateChange
epics::pvAccess::ServerChannel::unregisterRequest
locks channel mutex
epics::pvAccess::ServerChannelRPCRequesterImpl::destroy
locks RPC requester mutex
epics::pvAccess::ServerChannelRPCRequesterImpl::send
oh my was this bad code! It isn't clear what
lock()/unlock() are guarding, but let them
continue as before, but don't swallow any exception,
or unlock twice.
Swallowing exceptions here hides some of our errors.
Particularly unrecoverable introspectionRegistry misses.
Instead, propagate up to Thread and close connection.
Calling code can't deal with "cache" miss,
which indicates either a protocol error (use of invalid ID),
or a logic error on our part (didn't save ID).
In either case, better to throw an exception than to crash.
The DEBUG flag must be 0 when running under a test harness to
remove extraneous output which can confuse the output parser.
This commit also converts some throws into tests, and adds
more test diagnostic messages.
Move from global to ServerContext, apply lock, and
properly test if no provider has claimed, eg an attempt
at unadvertised PV (still wouldn't work, but at least
wouldn't crash).
Michael Davidsaver