espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
603 stars 255 forks source link

mqtt: Remove MQTT_API_LOCK/MQTT_API_UNLOCK in esp_mqtt_abort_connection (IDFGH-4271) #175

Closed AxelLin closed 3 years ago

AxelLin commented 3 years ago

All the callers except the one in esp_transport_poll_read() error path have held the lock before calling esp_mqtt_abort_connection(). So Just add MQTT_API_LOCK/MQTT_API_UNLOCK before calling esp_mqtt_abort_connection() in esp_transport_poll_read() error path. With this change, we can reduce a level of lock when calling esp_mqtt_abort_connection in most cases.

Signed-off-by: Axel Lin axel.lin@ingics.com

david-cermak commented 3 years ago

I would prefer keeping the lock in the esp_mqtt_abort_connection(), just to be on a safe side when adding new features/functions/APIs which would abort connection. I don't think we should be too concerned about performance when closing the connection. Open to discussion, though. but if you agree, you can just close the PR. Thanks!