-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Crash in WiFiScanClass::_scanDone() with negative scan result #8952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I'm looking into this code a bit more and there are lots of really strange things here. For example: void * WiFiScanClass::_getScanInfoByIndex(int i)
{
if(!WiFiScanClass::_scanResult || (size_t) i >= WiFiScanClass::_scanCount) {
return 0;
}
return reinterpret_cast<wifi_ap_record_t*>(WiFiScanClass::_scanResult) + i;
} A very tricky way to not check for One of the return statements in if(WiFiGenericClass::waitStatusBits(WIFI_SCAN_DONE_BIT, 10000)){
return (int16_t) WiFiScanClass::_scanCount;
} So this clearly shows it should be considered a signed int, as does I can't find what has been changed recently, so no idea why it is now completely unusable where it was running fine on previous ESP-IDF5.1 builds I made. Edit: |
Not sure if it can be used on array allocations, but isn't the simplest fix to just do |
@SuGlider Can you please take a look as you are now investigation WiFi changes? Thanks |
@TD-er - Please provide a sketch that I can use to reproduce this issue. |
It is not clear to me how and why you need to use This fuction will be called from the WiFi Event Callback only when it gets |
It is being called when the scan is done. I can't not call it. It's called from: esp_err_t WiFiGenericClass::_eventCallback(arduino_event_t *event) with event ID |
Sorry, you replied fast... :-) -- I was editing my reply - including exactly what you wrote. |
I need some sketch that I can use to reproduce this issue and find out what could be its cause. |
But what if the scan is called again while busy or failed? I have to think about some short sketch to reproduce, as you probably don't think "the entire ESPEasy project" is "a short sketch" ;) |
In my opinion, IF the sketch calls it, the code must make sure that the previous array was deleted, freeing the memory, and that a previous scan process has succeeded. |
I don't call it, it is called from the Arduino callback function. |
Can you point me out the code in the ESPEasy project where WiFi Scan takes place? |
Reading the ESPEasy Project code, I see that the issue seems to be related to Async WiFi Scan... |
It happened while working on the PR for IDF5.1 |
Ehh wow, you read through all that WiFi code in under 6 minutes? Edit: |
I just thought of this recent change I made after reporting this issue: This disconnect code is now wrapped in |
Ok, I've read some other pieces of code here and there, but it is not clear why the issue happens. I may try to help it by describing how WiFi Async Scan should work:
5- From one WiFi Scan to another it is necessary to release the array memory by calling |
But as I mentioned before, the arduino-esp32/libraries/WiFi/src/WiFiScan.cpp Lines 109 to 121 in b811ea4
N.B. |
I can't see a reason for making There are only two places when in the line: IDF documentation says: Attention: This API can only be called when the scan is completed, otherwise it may get wrong value.Also in |
In other words, Any scketch that wants to scan WiFi shall folow the right sequence as in the Arduino WiFi example (although it doesn't use the Asynch mode). |
A valid code should be like this: if (WiFi.scanComplete() > 0) _scanDone(); |
Yep, but like I said, I don't call this |
I need a sketch that can reproduce the issue. At this point I can't dig in. The Line 113 is not in the _scanDone() code. Maybe PlatformIO has some different core code for it? |
I will try to make a small sketch for it. |
Ehhm are we looking at the same code? arduino-esp32/libraries/WiFi/src/WiFiScan.cpp Lines 109 to 121 in 51cb927
|
I see... My bad. I don't remember where I looked this line 113. I'll wait for the sketch that demonstrates the issue. Thanks! |
I have created an example that uses Async WiFi Scanning. Do you want to try this change to check if it may fix ESPEasy Project? |
The issue fixed in Added Note:I think that this change may solve some issue related to |
Had a few personal issues the past few days, but will now try to see if I can still reproduce and hopefully no longer reproduce when applying your suggestions. |
Tried it and it still crashing.
|
@TD-er - This seems to be a new issue. I'd say that it would need a new investigation and debugging. It would be better if you could isolate the issue and create a small sketch that can reproduce it. |
Hello, Due to the overwhelming volume of issues currently being addressed, we have decided to close the previously received tickets. If you still require assistance or if the issue persists, please don't hesitate to reopen the ticket. Thanks. |
Just came across this exact issue. I have just started to get negative2 response from a very simple " int n = WiFi.scanNetworks();". (n a huge code too complex to add here , but available on Github if you really need it. |
Answer (for me at least) was to add WiFi.disconnect(false); in the case I do not find the expected ssid initially. |
`WIFI_SCANNING_BIT` may not always be cleared on timeout. Also `_scanCount` will only be cleared if there is a `_scanResult`. See: espressif#8952
Board
Any
Device Description
Not related to device, but crash happens on ESP32-C2 and -C3
Hardware Configuration
Not HW related
Version
latest master (checkout manually)
IDE Name
PlatformIO
Operating System
Windows 11
Flash frequency
40MHz
PSRAM enabled
no
Upload speed
115200
Description
I get crashes related to the new-operator when processing results from a WiFi scan
It seems to be caused by the code in this function:
arduino-esp32/libraries/WiFi/src/WiFiScan.cpp
Lines 109 to 121 in b811ea4
As can be seen, there is no check for negative scan results as the type of
_scanCount
is anuint16_t
.However the result of a scan can be negative, so maybe there is some conversion somewhere to this unsigned value and thus resulting in an attempt to allocate 65k elements of
wifi_ap_record_t
, which does fail at least on a C2.Sketch
Debug Message
Other Steps to Reproduce
No response
I have checked existing issues, online documentation and the Troubleshooting Guide
The text was updated successfully, but these errors were encountered: