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

Make POSIX-like writes write everything when blocking #5466

Merged
merged 2 commits into from Nov 27, 2017

Conversation

Projects
None yet
8 participants
@kjbracey-arm
Contributor

kjbracey-arm commented Nov 9, 2017

FileHandle and TCPSocket both have POSIX-like write calls. Both of them only ever write as much as currently fits into the buffer or underlying stack in one go.

It would be advantageous if we were to follow the spirit of POSIX a bit more closely and make those write calls write everything when in blocking mode - POSIX does try to write everything when blocking, and only returns early if interrupted by a signal.

As we don't have signals, we can guarantee to always write everything when blocking (if no errors), and this could make callers' lives easier.

First commit here updates TCPSocket for testing. A commit for UARTSerial.cpp and FileHandle.h will follow.

See commit message for the TCPSocket for more excruciating detail and POSIX spec quotes.

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Nov 9, 2017

First draft, not ready yet.

@SeppoTakalo, @juhaylinen - maybe the tests your working on should be written on the assumption TCPSocket works like this?

review thoughts? @geky, @bogdanm, @mikaleppanen

@geky

geky approved these changes Nov 9, 2017

+1

@mikaleppanen

This comment has been minimized.

Contributor

mikaleppanen commented Nov 10, 2017

Looks good to me.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 13, 2017

@kjbracey-arm Is this ready yet?

@theotherjimmy theotherjimmy added needs: work and removed needs: CI labels Nov 13, 2017

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Nov 20, 2017

I'm happy that this can proceed now.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 20, 2017

/morph build

@0xc0170 0xc0170 added the needs: CI label Nov 20, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Nov 20, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 20, 2017

IAR related failures [Error] TCPSocket.cpp@127,0: [Pe852]: expression must be a pointer to a complete object type please review

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Nov 20, 2017

kjbracey-arm added some commits Nov 9, 2017

Make TCPSocket send all data when blocking
Previously, send() was somewhat soft - it only ever made one send
call to the underlying stack, so it would typically take as much data
as would fit in the buffer, and only block if it was unable to write
anything.

This is not the intent of a POSIX socket/filehandle write. It should try
to send everything if blocking, and only send less if interrupted by a
signal:

 - If the O_NONBLOCK flag is clear, write() shall block the calling
   thread until the data can be accepted.

 - If the O_NONBLOCK flag is set, write() shall not block the thread.
   If some data can be written without blocking the thread, write()
   shall write what it can and return the number of bytes written.
   Otherwise, it shall return -1 and set errno to [EAGAIN].

This "send all" behaviour is of slightly limited usefulness in POSIX, as
you still usually have to worry about the interruption possibility:

  - If write() is interrupted by a signal before it writes any data, it
    shall return -1 with errno set to [EINTR].

  - If write() is interrupted by a signal after it successfully writes
    some data, it shall return the number of bytes written.

But as mbed OS does not have the possibility of signal interruption, if we
strengthen send to write everything, we can make applications' lives
easier - they can just do "send(large amount)" confident that it will
all go in one call (if no errors).

So, rework to make multiple sends to the underlying stack, blocking as
necessary, until all data is written.

This change does not apply to recv(), which is correct in only blocking until
some data is available:

 - If O_NONBLOCK is set, read() shall return -1 and set errno to [EAGAIN].

 - If O_NONBLOCK is clear, read() shall block the calling thread until some
   data becomes available.

 - The use of the O_NONBLOCK flag has no effect if there is some data
   available.
Make UARTSerial send all data when blocking
Previously, write() was somewhat soft - it only ever made one attempt to
wait for buffer space, so it would take as much data as would fit in the
buffer in one call.

This is not the intent of a POSIX filehandle write. It should try to
send everything if blocking, and only send less if interrupted by a
signal:

 - If the O_NONBLOCK flag is clear, write() shall block the calling
   thread until the data can be accepted.

 - If the O_NONBLOCK flag is set, write() shall not block the thread.
   If some data can be written without blocking the thread, write()
   shall write what it can and return the number of bytes written.
   Otherwise, it shall return -1 and set errno to [EAGAIN].

This "send all" behaviour is of slightly limited usefulness in POSIX, as
you still usually have to worry about the interruption possibility:

  - If write() is interrupted by a signal before it writes any data, it
    shall return -1 with errno set to [EINTR].

  - If write() is interrupted by a signal after it successfully writes
    some data, it shall return the number of bytes written.

But as mbed OS does not have the possibility of signal interruption, if we
strengthen write to write everything, we can make applications' lives
easier - they can just do "write(large amount)" confident that it will
all go in one call (if no errors).

So, rework to make multiple writes to the buffer, blocking as necessary,
until all data is written.

This change does not apply to read(), which is correct in only blocking until
some data is available:

 - If O_NONBLOCK is set, read() shall return -1 and set errno to [EAGAIN].

 - If O_NONBLOCK is clear, read() shall block the calling thread until some
   data becomes available.

 - The use of the O_NONBLOCK flag has no effect if there is some data
   available.

@kjbracey-arm kjbracey-arm force-pushed the kjbracey-arm:write_all branch to 9678c80 Nov 21, 2017

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Nov 21, 2017

Pointer arithmetic on void pointer corrected.

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Nov 21, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 21, 2017

/morph build

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Nov 22, 2017

We did some CI work last night, might have disrupted this build. Rekicking off.
/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Nov 22, 2017

Build : SUCCESS

Build number : 577
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5466/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Nov 24, 2017

@theotherjimmy theotherjimmy merged commit b9c3003 into ARMmbed:master Nov 27, 2017

6 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kjbracey-arm kjbracey-arm deleted the kjbracey-arm:write_all branch Dec 5, 2017

@kjbracey-arm kjbracey-arm referenced this pull request Nov 9, 2018

Merged

Add TLSSocket example #11

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