108 lines
4.2 KiB
Plaintext
108 lines
4.2 KiB
Plaintext
=== on 21 Nov 2014, 15:07:44 Freddy Mallet wrote:
|
|
My 2 cents [~ann.campbell.2]:
|
|
|
|
* tag "multi-threading" is missing
|
|
* moreover the main purpose of the rule is the following one: as soon as Enter() and Exit() statements are not called in the same method, fully understanding when those methods are really called at run time might be tricky, that's why we advise as a good practice to locate them in the same method. So for me, this rule more relates to a maintainability issue.
|
|
|
|
=== on 24 Nov 2014, 14:07:22 Ann Campbell wrote:
|
|
\[~freddy.mallet] when Enter() and Exit are not called in the same method, there's no guarantee that they're both called.
|
|
|
|
=== on 25 Nov 2014, 10:23:33 Freddy Mallet wrote:
|
|
\[~ann.campbell.2] the code might be perfectly designed to be sure that method Enter() and Exit() are called as expected even when they are not located in the same method. But our rule engine is not smart enough to figure out when this is the case or not. That's why for me this rule relates to a coding best practice because by definition is they are not located in the same methods and even if the code is perfectly correct, this will require an extra effort for a developer to inject a change.
|
|
|
|
=== on 1 Apr 2015, 17:47:33 Ann Campbell wrote:
|
|
To follow up on this conversation, I've added a case (try/catch/finally) to this method & reverted SQALE to Reliability/Synchronization
|
|
|
|
=== on 11 May 2015, 11:38:11 Dinesh Bolkensteyn wrote:
|
|
\[~ann.campbell.2] If we are anyway enforcing that the ``++Monitor.Enter()++`` and ``++Monitor.Exit()++`` calls should be done from the same method, then why not simply ban the direct usage of the ``++Monitor++`` class and force the usage ``++lock (...) {}++`` directives instead?
|
|
|
|
|
|
http://stackoverflow.com/questions/4978850/monitor-vs-lock
|
|
|
|
=== on 21 May 2015, 13:45:47 Ann Campbell wrote:
|
|
\[~dinesh.bolkensteyn] I don't really understand how this SO post is an argument for that.
|
|
|
|
=== on 22 May 2015, 12:51:00 Dinesh Bolkensteyn wrote:
|
|
Well [~ann.campbell.2] I fail to see this why you would want to write the compliant solution:
|
|
|
|
|
|
----
|
|
class MyClass {
|
|
object lock = new object();
|
|
|
|
public void DoTheThing() {
|
|
try {
|
|
Monitor.Enter(lock);
|
|
// ...
|
|
} catch (ExceptionType e) {
|
|
//...
|
|
} finally {
|
|
Monitor.Exit(lock);
|
|
}
|
|
}
|
|
----
|
|
|
|
instead of the much simpler:
|
|
|
|
|
|
----
|
|
class MyClass {
|
|
object lock = new object();
|
|
|
|
public void DoTheThing() {
|
|
lock (lock)
|
|
{
|
|
}
|
|
}
|
|
----
|
|
|
|
=== on 22 May 2015, 13:01:31 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] I agree with [~dinesh.bolkensteyn] on this one. The ``++lock++`` block is equivalent to the following:
|
|
|
|
----
|
|
bool lockWasTaken = false;
|
|
var temp = obj;
|
|
try
|
|
{
|
|
Monitor.Enter(temp, ref lockWasTaken);
|
|
{ body }
|
|
}
|
|
finally
|
|
{
|
|
if (lockWasTaken) Monitor.Exit(temp);
|
|
}
|
|
----
|
|
|
|
So why not use the language construct if there is one for exactly this?
|
|
|
|
=== on 22 May 2015, 14:36:53 Ann Campbell wrote:
|
|
Are we talking about a new rule or a change to the recommendation in this one?
|
|
|
|
=== on 22 May 2015, 15:04:17 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] I would just change the recommended solution. And update the description saying that the easiest way of achieving the aim is to use the ``++lock++`` statement.
|
|
|
|
|
|
BTW the message and the descriptions seems to go after different things, and that is causing the confusion here. [~dinesh.bolkensteyn]?
|
|
|
|
=== on 22 May 2015, 18:49:21 Ann Campbell wrote:
|
|
\[~tamas.vajk] and [~dinesh.bolkensteyn] I've updated the description and compliant example. I've also moved the messages up from the Java subtask. See if they suit you better.
|
|
|
|
=== on 26 May 2015, 10:07:59 Tamas Vajk wrote:
|
|
Looks good.
|
|
|
|
=== on 27 May 2015, 08:19:20 Dinesh Bolkensteyn wrote:
|
|
This RSPEC was initially created to re-implement a Gendarme rule: DoNotUseLockedRegionOutsideMethodRule
|
|
|
|
=== on 29 May 2015, 14:26:30 Dinesh Bolkensteyn wrote:
|
|
http://www.mono-project.com/docs/tools+libraries/tools/gendarme/rules/concurrency/#donotuselockedregionoutsidemethodrule
|
|
|
|
=== on 1 Jun 2015, 08:16:25 Dinesh Bolkensteyn wrote:
|
|
\[~ann.campbell.2] Please review the description
|
|
|
|
=== on 1 Jun 2015, 17:30:08 Ann Campbell wrote:
|
|
looks good [~dinesh.bolkensteyn]. Thanks.
|
|
|
|
=== on 11 Jun 2015, 18:41:25 Ann Campbell wrote:
|
|
CodePro: Call Lock Without Unlock
|
|
|