You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2023/01/14 22:14:34 UTC
[GitHub] [nuttx] jlaitine opened a new pull request, #8114: Revert "arch: Don't free the context if the reference doesn't equal z…
jlaitine opened a new pull request, #8114:
URL: https://github.com/apache/nuttx/pull/8114
…ero"
struct stm32_i2c_inst_s instance is allocated on every call to stm32_i2cbus_initialize, and that instance is supposed to be deleted on every call to stm32_i2cbus_uninitialize.
The "refs" counter just keeps track on when the last one is deleted, and everything is unregisterd/disabled after that.
This reverts commit 8098c80338eacf0649e3f08c6faff27bdd769334.
## Summary
I suggest reverting this patch.
Memory leak issue found a constant memory leak on an st32f7 platform which frequently calls stm32_i2cbus_initialize and stm32_i2cbus_uninitialize
The whole malloc in these drivers is IMHO useless, for more proper cleanup, something like this can be considered:
https://github.com/tiiuae/nuttx/pull/80/commits/d4db1ed31ffc07d90eeffee205f4ad64239d66ea
## Impact
Fixes memory leak &eventually out-of-memory condition when a driver initilization fails and is re-tried periodically for some time.
## Testing
Tested on stm32f7 (pixhawk 5x)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #8114: Revert "arch: Don't free the context if the reference doesn't equal z…
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #8114:
URL: https://github.com/apache/nuttx/pull/8114
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8114: Revert "arch: Don't free the context if the reference doesn't equal z…
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #8114:
URL: https://github.com/apache/nuttx/pull/8114#issuecomment-1383129674
Ok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8114: Revert "arch: Don't free the context if the reference doesn't equal z…
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #8114:
URL: https://github.com/apache/nuttx/pull/8114#issuecomment-1383048103
@jlaitine I can't see why the memory leak happen before reverting, could you explain more?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jlaitine commented on pull request #8114: Revert "arch: Don't free the context if the reference doesn't equal z…
Posted by GitBox <gi...@apache.org>.
jlaitine commented on PR #8114:
URL: https://github.com/apache/nuttx/pull/8114#issuecomment-1383073252
> @jlaitine I can't see why the memory leak happen before reverting, could you explain more?
If there are more than one client using it already, and another driver initialization fails periodically, the refs counter is > 0. But after the patch the driver allocates it's own structure on every call to initialize, but doesn't any more free it on uninitialize.
The previous implementation was correct (although it is silly to malloc in the first place, the malloced struct just stores two pointers and no client specific data...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org