Skip to content

bugfix: multiple issues in i2c_dev_delete_mutex #17

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.

Sign up for GitHub

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

Merged
UncleRus merged 2 commits into from
Jul 9, 2026
Merged

bugfix: multiple issues in i2c_dev_delete_mutex #17

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62fbaa8
bugfix: multiple issues in i2c_dev_delete_mutex
Jul 9, 2026
4350faa
fix astyle
Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 68 additions & 64 deletions i2cdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,6 @@ static esp_err_t register_device(i2c_dev_t *dev)
return ESP_ERR_NO_MEM;
}

// Helper to deregister a device
static void deregister_device(i2c_dev_t *dev)
{
if (!dev)
return;
int port = dev->port;
if (port >= I2C_NUM_MAX)
return;
for (int i = 0; i < CONFIG_I2CDEV_MAX_DEVICES_PER_PORT; i++)
{
if (active_devices[port][i] == dev)
{
active_devices[port][i] = NULL;
ESP_LOGV(TAG, "[0x%02x at %d] Deregistered device from slot %d", dev->addr, port, i);
return;
}
}
}

esp_err_t i2cdev_init(void)
{
ESP_LOGV(TAG, "Initializing I2C subsystem...");
Expand Down Expand Up @@ -210,72 +191,95 @@ esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev)

ESP_LOGV(TAG, "[0x%02x at %d] Deleting device mutex and cleaning up resources", dev->addr, dev->port);

// Remove device from bus if handle exists
if (dev->dev_handle)
// If we cannot safely touch the port state (bad port index, or the
// subsystem was never initialized), there is no bus handle to release
// and no ref_count to decrement. Just drop the device mutex.
if (dev->port >= I2C_NUM_MAX || !i2c_ports[dev->port].lock)
{
if (dev->port >= I2C_NUM_MAX)
{
ESP_LOGE(TAG, "[0x%02x at %d] Invalid port %d, skipping port-scoped cleanup", dev->addr, dev->port, dev->port);
}
else
{
ESP_LOGW(TAG, "[0x%02x at %d] I2C subsystem not initialized, skipping port-scoped cleanup", dev->addr, dev->port);
}
if (dev->mutex)
{
vSemaphoreDelete(dev->mutex);
dev->mutex = NULL;
}
return ESP_OK;
}

i2c_port_state_t *port_state = &i2c_ports[dev->port];

// Take the port mutex once and run the whole cleanup under it.
if (xSemaphoreTake(port_state->lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) != pdTRUE)
{
ESP_LOGE(TAG, "[0x%02x at %d] Could not take port mutex for cleanup", dev->addr, dev->port);
return ESP_ERR_TIMEOUT;
}

// Capture whether this device was actually added to the bus before
// we clear the field.
bool was_on_bus = (dev->dev_handle != NULL);

// 1) Remove the device from the bus, if it was added
if (was_on_bus)
{
ESP_LOGV(TAG, "[0x%02x at %d] Removing device handle %p from bus", dev->addr, dev->port, dev->dev_handle);
esp_err_t rm_res = i2c_master_bus_rm_device((i2c_master_dev_handle_t)dev->dev_handle);
if (rm_res != ESP_OK)
{
ESP_LOGW(TAG, "[0x%02x at %d] Failed to remove device handle: %s", dev->addr, dev->port, esp_err_to_name(rm_res));
// Continue with cleanup despite error
}
dev->dev_handle = NULL;
}

// Deregister the device
deregister_device(dev);

// Update port reference count if port is valid
if (dev->port < I2C_NUM_MAX)
// 2) Deregister from active_devices[] under the same port mutex
for (int i = 0; i < CONFIG_I2CDEV_MAX_DEVICES_PER_PORT; i++)
{
// Check if port mutex is initialized
if (!i2c_ports[dev->port].lock)
if (active_devices[dev->port][i] == dev)
{
ESP_LOGW(TAG, "[0x%02x at %d] Port mutex not initialized, skipping ref_count update", dev->addr, dev->port);
// Continue with cleanup - just skip the mutex-protected section
active_devices[dev->port][i] = NULL;
ESP_LOGV(TAG, "[0x%02x at %d] Deregistered device from slot %d", dev->addr, dev->port, i);
break;
}
else if (xSemaphoreTake(i2c_ports[dev->port].lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) == pdTRUE)
}

// 3) Decrement ref_count; delete the bus if this was the last device
if (was_on_bus && port_state->installed && port_state->ref_count > 0)
{
port_state->ref_count--;
ESP_LOGV(TAG, "[Port %d] Decremented ref_count to %" PRIu32, dev->port, port_state->ref_count);

if (port_state->ref_count == 0)
{
// Only decrement ref_count if THIS device was actually added to the bus
if (i2c_ports[dev->port].installed && i2c_ports[dev->port].ref_count > 0 && dev->dev_handle != NULL)
ESP_LOGI(TAG, "[Port %d] Last device removed, cleaning up port's bus", dev->port);
if (port_state->bus_handle)
{
i2c_ports[dev->port].ref_count--;
ESP_LOGV(TAG, "[Port %d] Decremented ref_count to %" PRIu32, dev->port, i2c_ports[dev->port].ref_count);

// If last device on this port, delete the bus
if (i2c_ports[dev->port].ref_count == 0)
ESP_LOGI(TAG, "[Port %d] Deleting bus handle %p", dev->port, port_state->bus_handle);
esp_err_t del_bus_res = i2c_del_master_bus(port_state->bus_handle);
if (del_bus_res != ESP_OK)
{
ESP_LOGI(TAG, "[Port %d] Last device removed, cleaning up THIS port's bus", dev->port);
// Just clean up this port's bus
if (i2c_ports[dev->port].bus_handle)
{
ESP_LOGI(TAG, "[Port %d] Deleting bus handle %p", dev->port, i2c_ports[dev->port].bus_handle);
esp_err_t del_bus_res = i2c_del_master_bus(i2c_ports[dev->port].bus_handle);
if (del_bus_res != ESP_OK)
{
ESP_LOGE(TAG, "[Port %d] Failed to delete master bus: %s", dev->port, esp_err_to_name(del_bus_res));
}
i2c_ports[dev->port].bus_handle = NULL;
}
i2c_ports[dev->port].installed = false;
i2c_ports[dev->port].sda_pin_current = -1;
i2c_ports[dev->port].scl_pin_current = -1;
ESP_LOGE(TAG, "[Port %d] Failed to delete master bus: %s", dev->port, esp_err_to_name(del_bus_res));
}
port_state->bus_handle = NULL;
}
else if (dev->dev_handle == NULL)
{
ESP_LOGV(TAG, "[0x%02x at %d] Device was never added to bus, skipping ref_count decrement", dev->addr, dev->port);
}
xSemaphoreGive(i2c_ports[dev->port].lock);
}
else
{
ESP_LOGW(TAG, "[0x%02x at %d] Could not take port mutex for ref_count update", dev->addr, dev->port);
port_state->installed = false;
port_state->sda_pin_current = -1;
port_state->scl_pin_current = -1;
}
}
else if (!was_on_bus)
{
ESP_LOGV(TAG, "[0x%02x at %d] Device was never added to bus, skipping ref_count decrement", dev->addr, dev->port);
}

xSemaphoreGive(port_state->lock);

// Delete the mutex itself last
// 4) Finally, drop the per-device mutex
if (dev->mutex)
{
vSemaphoreDelete(dev->mutex);
Expand Down