Skip to content
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.

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

RTEMS socket close() hack #169

Merged
merged 3 commits into from
Dec 2, 2020

Conversation

mdavidsaver
Copy link
Member

RTEMS allows blocking sockets to be interrupted by shutdown() (aka. esscimqi_socketBothShutdownRequired). However, a concurrent close() is a race which can leave a send()/recv() call permanently stuck! The right way to handle this (aside from fixing the stack) would be to sequence shutdown() -> exitWait() -> epicsSocketDestroy(). This is hard to handle since this must be done for both sender and receiver thread, which presents difficulties owing to how the Transport hierarchy is structured since Transport::close() can happen on either worker, or a user thread.

Rather than try to straighten this mess out properly, we add this "wait" in-between shutdown() and epicsSocketDestroy() to hopefully wait for the workers (or other worker) to return from send()/recv().

cf. #150

Doesn't pass. for RTEMS 4.x.  Not clear why.
Disable so that the remaining tests can be run.
@mdavidsaver
Copy link
Member Author

So far this problem has not manifest for (or hasn't been reported by) users. However, it does appear if the remote/ unit tests are run for RTEMS, and results in hung tests.

I don't think this will have ill effects for actual applications/IOCs.

I'm also disabling testChannelAccess for RTEMS as it has two failures which I'm not going to investigate at present.

   testChannelAccess.tap ..... 
  not ok 22 - void ChannelAccessIFTest::test_channel(): channel state change count should increase on the destroyed channel
  not ok 25 - void ChannelAccessIFTest::test_channel(): a destroy event was caught for the testing channel that was destroyed twice 
  Failed 4/152 subtests 
  	(less 12 skipped subtests: 136 okay)

@mdavidsaver mdavidsaver self-assigned this Dec 2, 2020
@mdavidsaver mdavidsaver added the bug label Dec 2, 2020
@mdavidsaver mdavidsaver merged commit 29c0656 into epics-base:master Dec 2, 2020
@mdavidsaver
Copy link
Member Author

Merging now so that CI passes when I update the modules/pvAccess in Base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant