Increase number of LWIP timers for MDNS (fixes #7333)

[gneverov]

The proximate cause of this issue here, where this memory allocation fails.

timeout = (struct sys_timeo *)memp_malloc(MEMP_SYS_TIMEOUT);

This raises two questions:

1. Why did the memory allocation fail?
2. Why wasn't the failure more fatal (e.g., crash instead of continue as usual)?

Why did the memory allocation fail?

LWIP uses statically sized arrays from which it allocates each type of struct. In this case we exhausted the static array for the timeout struct. The size of the array is determined by MEMP_NUM_SYS_TIMEOUT which has some preprocessor magic to calculate the maximum number of allocations needed, so we should never run out. However this macro only knows about stuff in lwip/core; it doesn't know about lwip/apps.

From lwip/apps, we use mdns. In the code for mdns, there is this well hidden comment:

You need to increase MEMP_NUM_SYS_TIMEOUT by one if you use MDNS!

So we need to manually increase the value of MEMP_NUM_SYS_TIMEOUT in lwip/core from 7 to 8 and unfortunately we can't use the magic macro calculation anymore.

But it doesn't end there! A different mdns file, allocates a further 3 timers per IP version. And unlike the first file, there doesn't appear to be a comment to increase MEMP_NUM_SYS_TIMEOUT to compensate.

So that is how the new value of MEMP_NUM_SYS_TIMEOUT is calculated and it is sufficient to fix the issue.

Why wasn't the failure more fatal?

When the memory allocation fails, the code calls LWIP_ASSERT, which in turn calls LWIP_PLATFORM_ASSERT, which is defined here:

#define LWIP_PLATFORM_ASSERT(x) do { if(!(x)) while(1); } while(0)

The problem is that the parameter, x, to LWIP_PLATFORM_ASSERT is not the assert predicate, but the assert message. The message string constant is statically non-null, so this whole macro simplifies into a no-op instead of an infinite loop. This means the assert after the memory allocation failure does nothing and the program continues normally instead of halting, which would be appropriate for this non-recoverable error.

The fix for this is of course trivial; it's not necessary to fix #7333, but it is useful to catch similar bugs earlier. Currently Adafruit does not maintain its own fork of pico-sdk, so I will submit the PR to Raspberry Pi. Unless someone thinks it is worthwhile to create the fork.

[DavePutz]

Tested and confirmed that this fixes issue #7333

[dhalbert]

@gneverov Could you change the base to 8.0.x and I will get this in the next 8.0.x release? Thanks!

[tannewt]

Thank you!

[gneverov]

BTW, it turns out the LWIP_PLATFORM_ASSERT secondary issue is already fixed in the upstream pico-sdk. Perhaps you want to update the circuitpython submodule.