From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches@freelists.org, Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org> Subject: Re: [PATCH v3] socket: evaluate buffer size in recv / recvfrom Date: Wed, 29 Aug 2018 13:16:51 +0300 [thread overview] Message-ID: <20180829101651.o3s2knsmr53dbgf6@esperanza> (raw) In-Reply-To: <1c4959e92e1c768bf1bdb63409f46089c5948cbe.1535476618.git.alexander.turenko@tarantool.org> On Tue, Aug 28, 2018 at 09:21:45PM +0300, Alexander Turenko wrote: > diff --git a/src/lua/socket.lua b/src/lua/socket.lua > index 06306eae2..04e99a668 100644 > --- a/src/lua/socket.lua > +++ b/src/lua/socket.lua > @@ -770,6 +774,44 @@ local function socket_send(self, octets, flags) > return tonumber(res) > end > > +-- Returns nil and sets EAGAIN when tries to determine the next datagram length > +-- and there are no datagrams in the receive queue. Nit: please limit the width of comments to 60-70 symbols - easier to read them that way. > +local function get_recv_size(self, size) > + local fd = check_socket(self) Nit: you only need fd in the !OSX branch below so I'd move this there. No point in calling check_socket when size is set, right? > + > + if size ~= nil then > + return size > + end > + > + if self.itype ~= get_ivalue(internal.SO_TYPE, 'SOCK_DGRAM') then > + return 512 > + end > + > + -- Determine the next datagram length. > + self._errno = nil > + if jit.os == 'OSX' then > + size = self:getsockopt('SOL_SOCKET', 'SO_NREAD') > + -- recv() with zero length buffer is always successful on Mac OS (at > + -- least for valid UDP sockets). The assignment of size to 1 below > + -- allows to handle 'no datagram' situation consistently: returning nil > + -- and setting EAGAIN. > + if size == 0 then > + size = 1 > + end I'm afraid there's an inherent race condition here. What if a new datagram of non-zero length arrives after you check the size here, but before you call recv? > + else > + local iflags = get_iflags(internal.SEND_FLAGS, {'MSG_TRUNC', 'MSG_PEEK'}) > + assert(iflags ~= nil) > + size = tonumber(ffi.C.recv(fd, nil, 0, iflags)) > + end > + > + if size == -1 then > + self._errno = boxerrno() > + return nil > + end > + > + return size > +end > + > local function socket_recv(self, size, flags) > local fd = check_socket(self) > local iflags = get_iflags(internal.SEND_FLAGS, flags) > @@ -778,7 +820,11 @@ local function socket_recv(self, size, flags) > return nil > end > > - size = size or 512 > + size = get_recv_size(self, size) > + if size == nil then > + return nil > + end > + > self._errno = nil > local buf = ffi.new("char[?]", size) >
next prev parent reply other threads:[~2018-08-29 10:16 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-28 18:21 Alexander Turenko 2018-08-29 10:16 ` Vladimir Davydov [this message] 2018-08-29 16:59 ` [tarantool-patches] " Alexander Turenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180829101651.o3s2knsmr53dbgf6@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=yaroslav.dynnikov@tarantool.org \ --subject='Re: [PATCH v3] socket: evaluate buffer size in recv / recvfrom' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox