[squid-users] FATAL: assertion failed: mem/PageStack.cc:159: "StoredNode().is_lock_free()"

[Alex Rousskov]

On 2024-07-03 09:27, Nishant Sharma wrote:

> I had reported the issue to Openwrt project as well and I understood 
> that in order to save space, squid was being compiled for mips16 instead 
> of mips32.

Sorry, I do not know why/how that fact is relevant to this discussion, 
but thank you for sharing it as it may be useful for others.


> Even after compiling it for mips32 (by adding a flag for the build 
> system), the error is still occuring as in the code in 
> `ipc/mem/PageStack.h` `uint64_t` is specified.

Glad to hear that: That assertion should not depend on those build flags 
AFAICT.


> I was able to compile by replacing `uint64_t` to `uint32_t` and squid 
> worked with workers > 1.

Where did you replace uint64_t with uint32_t? In IdSet::Node typedef? 
Any other changes? AFAICT, changing just IdSet::Node badly breaks the 
corresponding binary tree code because we hard-code the number of bits 
per leaf node (at least!) to be 64. I did not audit code for other 
dependencies.


> Further discussion on Openwrt issue tracker suggested [1] the following:
> 
> <quote>
> The posix definition is that lock ID is native integer. It should be 
> some conditional in ./configure style portability of squid to run on 
> (most) 32bit platforms.
> </quote>

It is possible that the above comment was negatively influenced by the 
previous misleading statement about Squid v4 having no uint64_t: "In 
code for version 4.x, there is no mention of uint64_t. It was introduced 
with 5.x." In reality, Squid has been using 64-bit integers since before 
Squid v3. It is neither practical nor necessary to remove uint64_t from 
Squid so there will be no corresponding conditionals in ./configure.

The shared binary tree (that contains the assertion) did not exist in 
v4, as we discussed earlier.


> Is there any change that we need to do in the configure script to check 
> for the availability of 64 bit atomic lock and use 32 bit lock if not 
> available?

It is technically possible (perhaps even without ./configure checks), 
but it would require adjusting complex shared tree code in the abcense 
of comprehensive ready-to-use tests. It is trivial to break that code. 
It is difficult to detect bugs. IMO, we should not expose ourselves to 
such risks in this case, especially since Squid uses 64-bit atomics in 
many other places: Supporting 32 bits in shared binary tree nodes is not 
going to remove the last frequently used 64-bit lock.


> Or may be document the fact that it is not advisable / possible to run 
> squid in SMP mode on such platforms that are not able to provide 64 bit 
> lock ID.

I believe your experiments with removing the assertion point in a rather 
different direction: If your tests do not suggest otherwise, we should 
downgrade that assertion to a startup warning. Let folks run Squid on 
platforms without 64-bit atomic locks (if they wish to do so), but warn 
them about an uncertain impact. Perhaps we can even convince ourselves 
that the impact can only be on performance (i.e., there can be no 
deadlocks due to mutexes).

Disclaimer: I do not know what "lock ID" is in this context.


HTH,

Alex.


> I tried to go through config.log and could see the following messages 
> which might or might not be related to this:
> 
> <config.log>
> ...
> ...
> ...
> 
> configure:46036: checking for uint64_t
> configure:46036: $? = 0
> configure:46036: result: yes
> 
> AND
> 
> configure:46027: checking for int64_t
> 
> configure:46027: mipsel-openwrt-linux-musl-g++ -std=c++17 -c 
> -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include  -Os -pipe -mno-branch-likely -mips32r2 -mtune=24kc -fno-caller-saves -fno-plt -fhonour-copts -msoft-float -fmacro-prefix-map=/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/build_dir/target-mipsel_24kc_musl/squid-6.10=squid-6.10 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -Wno-error -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include/libxml2 -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/include/fortify -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/include  conftest.cpp >&5
> configure:46027: $? = 0
> 
> configure:46027: mipsel-openwrt-linux-musl-g++ -std=c++17 -c 
> -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include  -Os -pipe -mno-branch-likely -mips32r2 -mtune=24kc -fno-caller-saves -fno-plt -fhonour-copts -msoft-float -fmacro-prefix-map=/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/build_dir/target-mipsel_24kc_musl/squid-6.10=squid-6.10 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -Wno-error -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/target-mipsel_24kc_musl/usr/include/libxml2 -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/usr/include -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/include/fortify -I/home/devuser/openwrt-23.05/arthur/mt7621/openwrt/staging_dir/toolchain-mipsel_24kc_gcc-12.3.0_musl/include  conftest.cpp >&5
> 
> conftest.cpp: In function 'int main()':
> conftest.cpp:324:67: warning: integer overflow in expression of type 
> 'int64_t' {aka 'long long int'} results in '-9223372036854775808' 
> [-Woverflow]
>    324 |                  < (int64_t) (((((int64_t) 1 << N) << N) - 1) * 
> 2 + 2))];
>        | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> conftest.cpp:323:26: error: size '-1' of array 'test_array' is negative
>    323 | static int test_array [1 - 2 * !((int64_t) (((((int64_t) 1 << 
> N) << N) - 1) * 2 + 1)
>        | ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    324 |                  < (int64_t) (((((int64_t) 1 << N) << N) - 1) * 
> 2 + 2))];
>        | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> configure:46027: $? = 1
> 
> configure: failed program was:
> | /* confdefs.h */
> | #define PACKAGE_NAME "Squid Web Proxy"
> | #define PACKAGE_TARNAME "squid"
> | #define PACKAGE_VERSION "6.10"
> | #define PACKAGE_STRING "Squid Web Proxy 6.10"
> | #define PACKAGE_BUGREPORT "https://bugs.squid-cache.org/"
> | #define PACKAGE_URL ""
> ...
> ...
> ...
> ...
> </config.log>
> 
> Regards,
> Nishant
> 
> * [1] 
> https://github.com/openwrt/packages/issues/24469#issuecomment-2203033868