Thanks for the feedback Jong-Min. Here are my thoughts about what you said:
1. Do you have data that suggests a significant perf overhead inflicted by PTHREAD_MUTEX_RECURSIVE? I know that on Windows the recursion count is a simple counter ? maintained with simple (RecursionCount++) and (RecursionCount--). It doesn?t even need Atomic increments/decrements. I wouldn?t expect that kind of counter overhead to be measurable or perceptible in IoTivity. The PTHREAD recursion counter cannot be much slower than the Windows counter. Or, did you refer to memory overhead rather than performance? Do you have examples for scenarios where debugging becomes harder with recursive locks? 2. I agree that sometimes we need to test on multiple platforms. But, when we can save some of that time and energy, especially *without paying a significant cost*, I strongly believe that driving towards consistent behavior for multiple platforms is a worthy goal. I am not aware of a significant cost for the change we are discussing here. 3. I agree this is possible. #1 and #2 are more important than #3. Does anyone else have thoughts about my proposal? Thanks! From: iotivity-dev-bounces at lists.iotivity.org [mailto:[email protected]] On Behalf Of ??? Sent: Monday, February 27, 2017 7:30 PM To: Daniel Mihai via iotivity-dev <iotivity-dev at lists.iotivity.org> Subject: Re: [dev] proposal to enable PTHREAD_MUTEX_RECURSIVE Hello Dan, For the following reasons, I oppose to enabling the PTHREAD_MUTEX_RECURSIVE for oc_mutex. Instead, any code that produces deadlock in Linux should be fixed. 1. While it is convenient to have consistent lock behavior across all platforms, IoTivity is meant to support a variety of platforms. Enabling a feature in all platforms because one platform supports it does not seem like a desirable approach to a problem. In general, recursive mutex is not a desirable solution to deadlock as it will increase overhead due to tracking issues and make it much harder to debug in case of a deadlock. 2. Again, while it may be convenient in terms of keeping consistency across platforms, it is not at all necessary that recursive mutex should be disabled for Windows. Of course, that means that when one uploads a patch, he/she needs to thoroughly test it across different platforms, but I think this is a reasonable requirement. 3. These issues should be solved by code refactoring or other code modification, not by changing basic mutex behavior for the platform. Thanks, Jong-Min Choi --------- Original Message --------- Sender : Daniel Mihai via iotivity-dev <iotivity-dev at lists.iotivity.org<mailto:iotivity-dev at lists.iotivity.org>> Date : 2017-02-28 00:58 (GMT+9) Title : [dev] proposal to enable PTHREAD_MUTEX_RECURSIVE Hi everyone, A few days ago I?ve learned that the Linux version of oc_mutex_lock is currently deadlocking if the current thread already owns the same lock. Some of the problems related to that behavior: 1. The Windows version of oc_mutex supports recursive-acquire. Having consistent lock behavior across all platforms supported by IoTivity allows us to leverage each other?s testing in a more consistent way. 2. Disabling oc_mutex recursivity on Windows would entail designing an IoTivity-specific lock - because the locks provided by the OS support recursion. 3. We?ve encountered these self-deadlocks for IOT-1861 and IOT-1488. So, I propose enabling the PTHREAD_MUTEX_RECURSIVE attribute for the Posix implementation of oc_mutex. Is everyone OK with this plan? I uploaded this code change in https://gerrit.iotivity.org/gerrit/#/c/17507/. Thanks, Dan _______________________________________________ iotivity-dev mailing list iotivity-dev at lists.iotivity.org<mailto:iotivity-dev at lists.iotivity.org> https://lists.iotivity.org/mailman/listinfo/iotivity-dev [cid:image001.gif at 01D29143.85476250] [http://ext.samsung.net/mail/ext/v1/external/status/update?userid=jminl.choi&do=bWFpbElEPTIwMTcwMjI4MDMzMDEyZXBjbXMxcDQ5NzE5Y2NhMTM5ZTJkZjM4YmY2ZGNjODJjZTY1NWMwMCZyZWNpcGllbnRBZGRyZXNzPWlvdGl2aXR5LWRldkBsaXN0cy5pb3Rpdml0eS5vcmc_] -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.iotivity.org/pipermail/iotivity-dev/attachments/20170228/78ce3a70/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: image001.gif Type: image/gif Size: 13402 bytes Desc: image001.gif URL: <http://lists.iotivity.org/pipermail/iotivity-dev/attachments/20170228/78ce3a70/attachment.gif>
