Repository navigation
Fix ESP32 I2C transaction locking - #353
Merged
Merged
Conversation
An out-of-range address was rejected by restart() after beginTransaction() had already taken the port lock. The state stayed disconnected, so the following endTransaction() returned early without unlocking, and every later transfer on that port blocked forever. The address is now checked before the lock is taken or the shared bus registers are saved. Rejecting before the lock means a failed begin no longer holds the port, so ending it must not touch a transfer owned by another task. The hardware port now records the task that owns the open transaction: - endTransaction(), restart(), writeBytes() and readBytes() return mode_mismatch when called by a task that does not own the transaction (including after an error has already released it). - A second beginTransaction() from the owning task returns mode_mismatch instead of deadlocking. - transactionWrite/Read/WriteRead() no longer call endTransaction() when beginTransaction() failed. Contract: do not call endTransaction() after a failed beginTransaction(). writeRegister8() with a mask (bitOn/bitOff) read and wrote the register in two transactions and released the lock in between, so a concurrent update of the same register from another task could be lost. The port mutex is now recursive and held across the read and the write. The bus sequence is unchanged (STOP between the read and the write). This serializes lgfx callers only; software (bit-bang) ports are not covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
Lock leak on an invalid address.
beginTransaction()took the port lock and then letrestart()reject an out-of-range address. The state stayedstate_disconnect, so the followingendTransaction()returned early without unlocking or restoring the saved registers. Every later transfer on that port then blocked forever (the same task deadlocks on the non-recursive mutex). The address is now checked before the lock is taken.Transaction owner. Rejecting before the lock means a failed begin no longer holds the port, so the cleanup after it must not touch a transfer that another task is running. The hardware port now records the task that owns the open transaction:
endTransaction(),restart(),writeBytes()andreadBytes()returnmode_mismatchwhen the calling task does not own the transaction. This also covers a task that keeps writing after an error has already released the port.beginTransaction()from the owning task returnsmode_mismatchinstead of deadlocking.transactionWrite()/transactionRead()/transactionWriteRead()no longer callendTransaction()whenbeginTransaction()failed.Contract: do not call
endTransaction()after a failedbeginTransaction().Atomic
bitOn()/bitOff().writeRegister8()with a mask read and wrote the register in two transactions and released the lock in between, so a concurrent update of the same register from another task could be lost. The port mutex is now recursive and held from the read to the write. The bus sequence is unchanged (a STOP between the read and the write), so devices see the same traffic as before. This serializes callers of this library only. Another driver sharing the bus can still transfer between the two, and software (bit-bang) ports keep the previous behavior.Checked
beginTransaction(port, 0x800)+endTransaction(), then a register read of an on-board device from another task. "Held" runs the same while another task keeps an open transaction on the port.endTransaction()returnsmode_mismatchBus scans, touch, PMIC/RTC and the IO expanders keep working with the change.
Not tested