[gendarme] Re: DecorateThreadsRule

On May 5, 2009, at 5:55 PM, Sebastien Pouliot wrote:

>
> Hello Jesse,
>
> On Mon, 2009-05-04 at 16:50 -0700, Jesse Jones wrote:
>> but I think it would be a
>> simpler model and one which would result in less stability under
>> changes if the rule verified that the threading model specified by  
>> the
>> code was used correctly.
>
> I'm not sure I'm parsing the "less stability under changes"
> correctly ?!?

Well, I meant to say more stability. :-) Formerly if the code was  
changed so that method A was no longer called from threaded code then  
A and potentially everything it called would have to be changed to  
remove the threaded attribute. Now this is no longer the case: A can  
be marked as thread safe even if it isn't actually used with threads.
>
>
>> What I'm thinking is to instead use:
>>
>> MultiThreadAttribute - The code is fully thread safe and can be used
>> concurrently with multiple threads without user locking.
>>
>> SerialThreadAttribute - The code can be used be used with any thread,
>> but concurrent access requires user locking.
>>
>> SingleThreadAttribute - The code executes under a single thread (and
>> the attribute is constructed with an optional name).
>>
>> MainThreadAttribute - The code is only safe to use within the main
>> thread.
>>
>> And the rule would not complain if code marked as threaded was not
>> used by a thread.
>
> So if I understand correctly, the rule would be checking for threading
> usage instead of checking if the attributes are making sense, right ?

It's more the reverse: the rule checks to see that the attributes are  
used correctly. It currently performs these checks:

* Thread entry points must use either [SingleThread("name")] or  
[MultiThread].

* Methods called by distinct threads must be [MultiThread] or  
[SerialThread].

* A target method can only be called if the source and target have  
identical threading models or the target is multi/serial threaded.

* An override of a base method or an implementation of an interface  
method must use the same threading model as the original method or  
[MultiThread].

* A delegate used with a threaded event must use the same threading  
model as the event or [MultiThread].

* A method used to construct a threaded delegate must use the same  
threading model as the delegate or [MultiThread].

* [SerialThread] cannot be applied to static methods and static  
methods of serial types do not inherit it from their types. (The  
rationale here is that there is normally nothing that can be used to  
serialize access to static methods other than the type which is a bad  
idea, see [http://bytes.com/groups/net-c/249277-dont-lock-type- 
objects]).
>
>
> If so then it makes sense to me, since it's closer to the goal (than  
> the
> attributes which are the tools).

Not quite sure that that means, but my goal currently is to provide a  
way for people to precisely specify the threading model supported by  
their code and to validate the specification as much as possible.
>
>
>
> Unrelated to the choice above, have you considered using less  
> attributes
> (or only one) and adding properties ?
>
> e.g. [Thread (ThreadSafety.MultiThreaded, Locking.NoneRequired)]
>
> enum ThreadSafety.[MultiThread | SingleThreaded | MainThreadOnly]
> enum Locking.[NoneRequired | CallerRequired]
>
> It makes it more obvious that they are all related (even more when
> there's only one ;-) but it's a bit harder to read from cecil (and
> rules).

I'm not terribly happy with having four attributes. I had considered  
using one attribute, but I didn't consider using properties like that.  
It's an interesting idea, but I'm going to have to think about it some.

   -- Jesse


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Gendarme" group.
To post to this group, send email to [EMAIL PROTECTED]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/gendarme?hl=en
-~----------~----~----~----~------~----~------~--~---