> On 6 May 2018, at 06:32, Willy Tarreau <[email protected]> wrote: > > Hi Pieter, > > On Sun, May 06, 2018 at 12:00:14AM +0200, PiBa-NL wrote: >> The parameters like server-address, port and timeout should be set before >> process_stream task is called to avoid the stream being 'closed' before it >> got initialized properly. This is most clearly visible when running with >> tune.lua.forced-yield=1.. So scheduling the task should not be done when >> creating the lua socket, but when connect is called. The error >> "socket: not yet initialised, you can't set timeouts." would then appear. >> >> Below code for example also shows this issue, as the sleep will >> yield the lua code: >> local con = core.tcp() >> core.sleep(1) >> con:settimeout(10) > > It seems to make sense indeed. I'll let Thierry check as he's the one who > really knows if there's a possible impact behind this.
I don’t see any impact, and I don’t remember the reason of the task woken during init and not during connect(). I agree with Willy, this patch make sense. In 1.6 version, the task is woken during the “hlua_socket_new” by the function “stream_new”. I test your code and ... segfault :-) The bug fix for the version 1.6 and 1.7 are available in attachment. The version 1.8 was not affected. Note that the following code: >> local con = core.tcp() >> core.sleep(1) >> con:settimeout(10) Can’t work in 1.6 and 1.7. It is bad :-( the function core.tcp() starts the task without Lua requirement (this is done in “stream_new()” function) and the new tcp connection disappear during the sleep(). I don’t see any work around. With 1.6, core.tcp() and conn:connect*() must be executed at the same time without Lua yield :-( The patch was not complex. I guess that I must remove the task from the run queue, and apply your patch to wake up the task after connect. But the workaround is easy, and I prefer to not modify this code. Any opinion about this ? From the 1.8 version, the task_wakeup is no longer called in the function “stream_new()”. The bug exists from the begining of the Lua socket, and it changes to its new form from the commit: 87787acf724eeaf413393b5fce0047ad74356815 MEDIUM: stream: make stream_new() allocate its own task So, I trust your patch, and it must be applied on the master branch and on the 1.8 branch. BR, Thierry
0001-BUG-MEDIUM-hlua-sockets-Segfault-raises-if-the-socke.1.6.patch
Description: Binary data
0001-BUG-MEDIUM-hlua-sockets-Segfault-raises-if-the-socke.1.7.patch
Description: Binary data
