Re: [PATCH] lua & threads

[Willy Tarreau]

On Thu, May 24, 2018 at 02:38:58PM +0200, Thierry Fournier wrote:
> I do not observe error during runtime, my only one problem is the
> compilation. I don't understand the impact of these modification,
> and so I can't test, because I don't known the impact on the
> polling.
(...)

Don't worry, I've finally fixed it, as it broke the build in 1.8.9
as well when threads were disabled. And indeed setting it to zero is
OK (I tested and that's fine since it's the same thing that is done
when you have a single thread).

Cheers,
Willy

Re: [PATCH] lua & threads

[Thierry Fournier]

> On 22 May 2018, at 19:03, Willy Tarreau  wrote:
> 
> Hi Thierry,
> 
> On Mon, May 21, 2018 at 07:58:01PM +0200, Thierry Fournier wrote:
>> Hi,
>> 
>> You will two patches in attachment.
>> 
>> - The first fix some Lua error messages
> 
> thanks, I've merged this one already.
> 
>> - The second fix a build error. This second should be reviewed because, I'm 
>> not
>>   so proud of solution :-) Note that this build error happens for compilation
>>   without threads on macosx.
> 
> In my opinion this one looks wrong. Apparently there's a special case for
> all_threads_mask when set to zero to indicate that no threads are enabled,
> and it bypasses any such checks, which is better than setting it to ULONG_MAX.
> 
> I *suspect* it doesn't have any impact for now, except that since code relies
> on !all_threads_mask it can progressively spread and break again later. So
> please check by setting it to 0UL and if it works that's OK.


I do not observe error during runtime, my only one problem is the
compilation. I don’t understand the impact of these modification,
and so I can’t test, because I don’t known the impact on the
polling.

The only one function impacted is “done_update_polling()” in proto/fd.h
which hangs during compilation without threads.

In other way, I so not like the remplacement of a variable by a define.
It can be create hard situation in the future, like which will work only
in thread case.

   long *ptr = _threads_mask


My patch have for goal to shows the error and not fix it.

BR,
Thierry

Re: [PATCH] lua & threads

[Willy Tarreau]

Hi Thierry,

On Mon, May 21, 2018 at 07:58:01PM +0200, Thierry Fournier wrote:
> Hi,
> 
> You will two patches in attachment.
> 
>  - The first fix some Lua error messages
 
thanks, I've merged this one already.

>  - The second fix a build error. This second should be reviewed because, I'm 
> not
>so proud of solution :-) Note that this build error happens for compilation
>without threads on macosx.

In my opinion this one looks wrong. Apparently there's a special case for
all_threads_mask when set to zero to indicate that no threads are enabled,
and it bypasses any such checks, which is better than setting it to ULONG_MAX.

I *suspect* it doesn't have any impact for now, except that since code relies
on !all_threads_mask it can progressively spread and break again later. So
please check by setting it to 0UL and if it works that's OK.

Thanks!
Willy

[PATCH] lua & threads

[Thierry Fournier]

Hi,

You will two patches in attachment.

 - The first fix some Lua error messages

 - The second fix a build error. This second should be reviewed because, I’m not
   so proud of solution :-) Note that this build error happens for compilation
   without threads on macosx.

BR,
Thierry



0001-MINOR-lua-Improve-error-message.patch
Description: Binary data


0002-BUG-MINOR-thread-build-The-variable-all_threads_mask.patch
Description: Binary data