<div dir="ltr"><div>Hello!</div><div><br></div><div>We have tested this fix in the idle state when there is not so much traffic on the UDP socket and also under the load. In both cases, the problem has disappeared, so I guess we could say that the fix works correctly, thanks!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">ср, 12 февр. 2020 г. в 17:13, Raimo Niskanen <<a href="mailto:raimo%2Berlang-questions@erlang.org">raimo+erlang-questions@erlang.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Feb 12, 2020 at 02:31:10PM +0300, Alexander Petrovsky wrote:<br>
> Awesome, thanks a lot :)<br>
> <br>
> I'll test this fix and in case of some problems, I will let you know!<br>
<br>
Great!<br>
<br>
I would apprecite if you could verify also if the fix works,<br>
so I can merge it into ourt maint and master.<br>
<br>
/ Raimo<br>
<br>
<br>
> <br>
> вт, 11 февр. 2020 г. в 21:01, Valentin Micic <<a href="mailto:v@micic.co.za" target="_blank">v@micic.co.za</a>>:<br>
> <br>
> > Thank you Raimo, much obliged.<br>
> > I am sure this will make Alexander very happy :-)<br>
> ><br>
> > V/<br>
> ><br>
> > > On 11 Feb 2020, at 17:21, Raimo Niskanen <<br>
> > <a href="mailto:raimo%2Berlang-questions@erlang.org" target="_blank">raimo+erlang-questions@erlang.org</a>> wrote:<br>
> > ><br>
> > > On Mon, Feb 10, 2020 at 04:23:37PM +0100, Raimo Niskanen wrote:<br>
> > >> I can reproduce it on a month old 'master'.<br>
> > >> We'll take a look at it...<br>
> > >><br>
> > >> Thank you for a pinpointed bug report!<br>
> > >> / Raimo<br>
> > ><br>
> > > I have a patch that fixes the problem:<br>
> > ><br>
> > > @@ -12609,12 +12609,8 @@ static ErlDrvSSizeT packet_inet_ctl(ErlDrvData<br>
> > e,<br>
> > > unsigned int cmd, char* buf,<br>
> > >               return ctl_error(EALREADY, rbuf, rsize);<br>
> > ><br>
> > >           if (packet_inet_input(udesc, desc->event) == 0) {<br>
> > > -             if (timeout == 0)<br>
> > > -                 async_error_am(desc, am_timeout);<br>
> > > -             else {<br>
> > > -                 if (timeout != INET_INFINITY)<br>
> > > -                     driver_set_timer(desc->port, timeout);<br>
> > > -             }<br>
> > > +                if (timeout != INET_INFINITY)<br>
> > > +                    driver_set_timer(desc->port, timeout);<br>
> > >           }<br>
> > >           return ctl_reply(INET_REP_OK, tbuf, 2, rbuf, rsize);<br>
> > >       }<br>
> > ><br>
> > > It should be possible to apply to any version.  The code lines I removed<br>
> > > have been around since we entered GitHub.<br>
> > ><br>
> > > The shortcut for timeout == 0 was flawed.  Right before this an async<br>
> > event<br>
> > > was enqueued, which is not cleared when taking this short way out, and<br>
> > the<br>
> > > packet_inet_input function had put the socket in the poll set, which the<br>
> > > shortcut also ignored.  So when the UDP packet arrives it is received and<br>
> > > delivery to the caller fails since there is no longer any caller - the<br>
> > caller<br>
> > > was already told {error, timeout}.  The packet gets wasted.<br>
> > ><br>
> > > This fix removes the shortcut for timeout == 0.  It would be possible to<br>
> > > instead fix the shortcut, which could improve latency slightly.  With<br>
> > this<br>
> > > fix code sets a timer for timeout == 0, which immediately times out<br>
> > > and then all internal state gets updated as it should.  The same code<br>
> > path<br>
> > > is used for all timeouts including 0.<br>
> > ><br>
> > > Again, thank you for the bug report and pinpointing.<br>
> > ><br>
> > > I will now create an internal ticket, write a test case, and prepare<br>
> > > for a patch...  We would prefer to just include this fix in the upcoming<br>
> > > OTP 22.3.<br>
> > ><br>
> > > Best regards<br>
> > > / Raimo Niskanen<br>
> > ><br>
> > ><br>
> > >><br>
> > >><br>
> > >> On Fri, Feb 07, 2020 at 04:50:54PM +0300, Alexander Petrovsky wrote:<br>
> > >>>><br>
> > >>>> From: Valentin Micic<br>
> > >>>><br>
> > >>>> I’ve run a couple of tests on my own, and managed to reproduce your<br>
> > >>>> “second! and only second!!” packet problem.<br>
> > >>>><br>
> > >>>> It happens under following circumstances:<br>
> > >>>><br>
> > >>>> 1) Socket (Rx) is instructed to provide a single packet as it arrives<br>
> > ( as<br>
> > >>>> per {active, once} );<br>
> > >>>><br>
> > >>>><br>
> > >>>> 2) After receiving this packet, a call to gen_udp:recv( Rx, 0, 0 ) is<br>
> > >>>> executed before another packet has ARRIVED to the kernel buffer;<br>
> > >>>><br>
> > >>>> Then the following diagram illustrates the problem:<br>
> > >>>><br>
> > >>>> Note:<br>
> > >>>>    - Tx is sending socket;<br>
> > >>>>    - Rx is receiving socket;<br>
> > >>>><br>
> > >>>> *Please note that sequence diagrams below are not completely<br>
> > accurate, but<br>
> > >>>> it is reasonably illustrative of what may have happened.*<br>
> > >>>><br>
> > >>>><br>
> > >>>><br>
> > >>>> The next sequence diagram depicts a situation when a non-zero value<br>
> > for<br>
> > >>>> gen_udp:recv/3, e.g. gen_udp:recv( Rx, 0, *10* ):<br>
> > >>>><br>
> > >>>><br>
> > >>>><br>
> > >>>><br>
> > >>>> Thus, it appears that gen_udp:recv( *Socket, Length*, *Timeout* ) is<br>
> > >>>> broken when *Timeout* is set to *0*.<br>
> > >>>> For all positive non-zero values for Timeout, it behaves properly (or<br>
> > as<br>
> > >>>> intuitively expected).<br>
> > >>>><br>
> > >>>> Therefore, this should either be fixed, or it should be documented in<br>
> > >>>> order to indicate that 0 must *not* be used as a value for the<br>
> > Timeout in<br>
> > >>>> gen_udp:recv/3 call.<br>
> > >>>><br>
> > >>>> Aside, I fail to see a benefit of using Timeout of value 0 when<br>
> > combined<br>
> > >>>> with {active, once}. The only time when zero value may appear to be<br>
> > useful<br>
> > >>>> is when server does need to process other messages, e.g. messages not<br>
> > >>>> originated by the socket, which would be the case when gen_server is<br>
> > used<br>
> > >>>> to support the socket. In my view (which is supported by substantial<br>
> > >>>> empirical evidence), gen_server is not the most optimal behaviour to<br>
> > use<br>
> > >>>> when implementing support for sockets.<br>
> > >>>><br>
> > >>>> Kind regards<br>
> > >>>><br>
> > >>>> V/<br>
> > >>>><br>
> > >>>><br>
> > >>> So, seems like there is some buggy behavior in the erlang VM for<br>
> > reading<br>
> > >>> UDP messages between switching from {active, once} to<br>
> > gen_udp:recv(socket,<br>
> > >>> 0, 0)? Lukas, Raimo could you help with that?<br>
> > >>><br>
> > >>> --<br>
> > >>> Петровский Александр / Alexander Petrovsky,<br>
> > >>><br>
> > >>> Skype: askjuise<br>
> > >>> Phone: +7 931 9877991<br>
> > >><br>
> > >><br>
> > >><br>
> > >><br>
> > >> --<br>
> > >><br>
> > >> / Raimo Niskanen, Erlang/OTP, Ericsson AB<br>
> > ><br>
> > > --<br>
> > ><br>
> > > / Raimo Niskanen, Erlang/OTP, Ericsson AB<br>
> ><br>
> ><br>
> <br>
> -- <br>
> Alexander Petrovsky<br>
<br>
-- <br>
<br>
/ Raimo Niskanen, Erlang/OTP, Ericsson AB<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr">Alexander Petrovsky</div></div></div></div></div></div>