 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
.NET Forum / Languages / C# / April 2008ReaderWriterLockSlim + Dispose?
One last question today :) Most locks I've used never had a Dispose (Monitor, ReaderWriterLock, etc). Looking at the ReaderWriterLockSlim's Dispose method its just cleaning up some Threading.WaitHandle's. Whenever I see a Dispose method I always want to call it after I'm done with the object. All examples I've ever found online don't use dispose with the ReaderWriterLockSlim. Making my classes that use this lock IDisposable for this reasion alone is a little frustrating, and makes the implementation a little harder (since I have to constantly check to see if the locks disposed, mainly when using it with a thread pool). Since only WaitHandles are getting closed, the GC will eventually clean them if I ignore the Disposing on the ReaderWriterLockSlim? Is there performance increases in using the disposable WaitHandles vs how other locks without Dispose() do it? Thanks :) I'm mostly just curious. NB > One last question today :) Most locks I've used never had a Dispose > (Monitor, ReaderWriterLock, etc). Looking at the > ReaderWriterLockSlim's Dispose method its just cleaning up some > Threading.WaitHandle's. Why do you write "just"? I would guess that for most classes that implement IDisposable, they "just" clean up some things. Are you implying that the ReaderWriterLockSlim.Dispose() is less important than for other classes that implement IDisposable? > Whenever I see a Dispose method I always want to call it after I'm > done with the object. Yes. > All examples I've ever found online don't use > dispose with the ReaderWriterLockSlim. Do you mean that the examples use ReaderWriterLockSlim but don't dispose it when the code is done with it? That's a bug. > Making my classes that use > this lock IDisposable for this reasion alone is a little frustrating, > and makes the implementation a little harder (since I have to > constantly check to see if the locks disposed, mainly when using it > with a thread pool). I don't understand why it's a problem. Why do you have to check to see if the lock is disposed? Why would it be disposed unless you're sure you're done with it (i.e. your own Dispose() method has been called)? > Since only WaitHandles are getting closed, the GC will eventually > clean them if I ignore the Disposing on the ReaderWriterLockSlim? Not necessarily. If the finalizer thread gets around to running, they will eventually get cleaned up, yes. But there's no guarantee that this will ever happen, or when it will happen. You should not design code that relies on that (other than implementing your own finalizer, of course). > Is > there performance increases in using the disposable WaitHandles vs how > other locks without Dispose() do it? It seems to me that the main difference is one of functionality. ReaderWriterLockSlim serves a very specific purpose. If you have need for that purpose, you are likely going to need some disposable synchronization object anyway, so using a lock implementation instead that itself needs disposing doesn't make things any worse with respect to disposal, but of course simplifies your life in that you don't have to reimplement all the stuff in ReaderWriterLockSlim. If you don't need the added functionality that ReaderWriterLockSlim provides, then you might as well use one of the simpler, non-disposable synchronization techniques. Alternatively, you could use the older ReaderWriterLock class, and accept the additional performance overhead it has. Pete On Apr 4, 5:11 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > Are you implying > that the ReaderWriterLockSlim.Dispose() is less important than for other > classes that implement IDisposable? Hehe, aye, that was my question. For example, if it was a file, and others are potentially wanting to use the file, then I'd rank high on "definally dispose when your done". But I've never really worked with these "WaitHandles". Are they low priority to close? Does the OS just create new ones when people request them, so it doesn't matter even a little bit if the one ReaderWriterLockSlim is using isn't closed for a few mins after its done with it? I only see a memory potential problem here, but its not like WaitHandles are images, no one really cares about a few bytes till GC collects it (unless its in some kind of loop). Again, I could be wrong, which is why I'm asking, basically what I'm missing. > Do you mean that the examples use ReaderWriterLockSlim but don't dispose > it when the code is done with it? That's a bug. http://msdn2.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx http://msdn2.microsoft.com/en-us/library/bb355025.aspx http://www.bluebytesoftware.com/blog/PermaLink,guid,c4ea3d6d-190a-48f8-a677-44a4 38d8386b.aspx To name a few. The 1st MSDN example displays the entire class with no dispose in it. If you google / google code search "ReaderWriterLockSlim" I don't think you'll be able to find a code snipplet or example that uses dispose with RWLS, which I found very strange. This made me believe that there is something I'm missing? Possible responsing I might of been expecting after posting was stuff like: "Ahh, its because all those examples are used in the main object, where 99.9% of locks are used. Since the application closes when your done with the object, that uses the RWLS, no one worries about using the dispose method for RWLS. Its just correctness for the other 0.1% of the time someone might be using it in a class that gets created thousands of times, and thus may want to clean it up after its done.". Or maybe a "Its a new class so people havn't learned how to use it correctly yet, I'd highly suggest always calling the Dispose, even if its in your main object; YOU DEFINALLY DON'T WANT THOSE WAITHANDLES FLOATING AROUND ANYLONGER THAN YOU HAVE TOO". Basically, for example, every code snipplet/example on say StreamReader has the Dispose method with it, why doesn't ReaderWriterLockSlim? > I don't understand why it's a problem. Why do you have to check to see if > the lock is disposed? Why would it be disposed unless you're sure you're > done with it (i.e. your own Dispose() method has been called)? There are reasions. For example, if your sychronizing a thread pool with RWLS's, then you can't do something like "stop all theads, its time to clean up" that I know of. A real life example could be, say your making a macroing program. Its designed to run multiple macros at the same time and are using the thread pool. Even if you know 5 macros are running (maybe in constant loops), it's time to clean up or shut down, you don't know how long they are going to take. Since the client made the macro(script) it could be doing some kind of huge loop and don't want to wait for it. Cleaning up could result in that thread trying to use the lock, which is annoying in the case of RWLS- if its the only disposable thing in the application. With the thread pool, any situation you don't want to wait for the threads to finish before cleaning up could result in crashes if you don't check if the lock is disposed before acquiring it. Yes yes, you probably don't want to use the thread pool in that type of situation, but not knowing when the threads are going to be done is somewhat common, even if you did know you don't always want to wait around for it to finish (expecially in the case of a thread pool where you can't interrupt/abort it, and who knows how long till it checks for a stop variable). > Not necessarily. If the finalizer thread gets around to running, they > will eventually get cleaned up, yes. But there's no guarantee that this > will ever happen, or when it will happen. You should not design code that > relies on that (other than implementing your own finalizer, of course). Which was another question. Does it really matter if these "WaitHandles" don't get cleaned up for a while? How does it affect other things if at all? (Other than maybe some memory that lingering till GC Collects / AppDomain unloads?). I don't see these WaitHandles being very big though memory wize though. > [...] If you don't need the added functionality that ReaderWriterLockSlim > provides, then you might as well use one of the simpler, non-disposable > synchronization techniques. Alternatively, you could use the older > ReaderWriterLock class, and accept the additional performance overhead it > has. I was thinking maybe the other implementations use WaitHandles and just don't clean them up? How many ways can there be for locks to wait on an event :) Hehe, my last posts last paragraph was more so asking this question. IE Maybe RWSL, without disposing, is very close to how Monitor works. It might not be but I was also fishing for any type of information with locks with the statment ;) :) Thanks for the comments, I'm still a little curious on information though about the topic. >> Do you mean that the examples use ReaderWriterLockSlim but don't >> dispose >> it when the code is done with it? That's a bug. > > http://msdn2.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx Well, as you probably can guess...I think this sample is incomplete, if not buggy. > http://msdn2.microsoft.com/en-us/library/bb355025.aspx This example stores the instance in a static variable, so IDisposable wouldn't apply. > http://www.bluebytesoftware.com/blog/PermaLink,guid,c4ea3d6d-190a-48f8-a677-44a4 38d8386b.aspx This doesn't even include compilable code, so I'd hardly say you should expect it to be complete. Lack of any specific code, IDisposable implementation or otherwise, shouldn't be in any way inferred to mean it can be left out in real code. > To name a few. The 1st MSDN example displays the entire class with no > dispose in it. If you google / google code search > "ReaderWriterLockSlim" I don't think you'll be able to find a code > snipplet or example that uses dispose with RWLS, which I found very > strange. This made me believe that there is something I'm missing? I can't say whether you're missing something. I do know that a quick Google search yielded this link: http://blogs.msdn.com/vancem/archive/2006/03/29/564854.aspx Check out the comments starting with Bill Menees's first comment. It doesn't pertain to ReaderWriterLockSlim per se, but it does perhaps give some insight as to why a class might have disposable stuff in it but not implement IDisposable, or why a class that does implement IDisposable might be considered "safe" to not dispose of. I'm not saying it's an answer as to why you shouldn't dispose ReaderWriterLockSlim. I'm still of the opinion that a class that implements IDisposable should be disposed of when you're done. But not everyone is necessarily going to agree with my viewpoint. > [...] Since the > client made the macro(script) it could be doing some kind of huge loop [quoted text clipped - 4 lines] > before cleaning up could result in crashes if you don't check if the > lock is disposed before acquiring it. I still don't see the problem. For me, it's a fundamental problem for you to start cleaning stuff up before things are done with them. In your example, why does the lock exist? Presumably, to ensure that the multiple macro-processing threads are properly synchronized with each other. So if you dispose that lock before those threads are done, and the threads simply avoid taking the lock if it's been disposed, now you have a situation in which the threads aren't synchronized. If that's safe now, it was safe before and you didn't need the lock. If it's not safe, then your decision to clean stuff up without terminating the threads created a synchronization bug. Either way, that's showing us that the hypothetical design is wrong in the first place. It's not an example of why disposing a lock class is a problem. In that particular example, you either need to decide whether you're going to interrupt the threads that need the lock, or you're going to delay cleanup of the lock until the threads are all done. You can't let the threads run _and_ clean up the lock right away. > [...] > Which was another question. Does it really matter if these > "WaitHandles" don't get cleaned up for a while? How does it affect > other things if at all? (Other than maybe some memory that lingering > till GC Collects / AppDomain unloads?). I don't see these WaitHandles > being very big though memory wize though. Well, generally speaking, it's not so much the size of the object as it is the size of the OS data structures that track the object. I suspect that these issues are _much_ alleviated since the old 16-bit Windows days. However, the OS does continue to provide dispose semantics for these objects, and I think it's unwise to make any assumptions that they are optional. Could you get away with it? Probably. The fact is, a lot of these sorts of things only wind up biting you some small fraction of the time you ignore them. But that doesn't mean it's generally acceptable design to do so. If anything, it's why you should _always_ be careful to follow the conventions imposed on you by the design, because it can be really difficult to reproduce potential problems at will. This means the problems often come up when you are least prepared or able to deal with them. > I was thinking maybe the other implementations use WaitHandles and > just don't clean them up? How many ways can there be for locks to > wait on an event :) Well, not all synchronization objects use events. Now, whether it's possible to write a synchronization object without using _any_ disposable unmanaged resources, I can't personally say. But inasmuch as managed code runs with the complete support of the .NET run-time, it seems plausible to me that there could be synchronization objects that have no need of being disposed. > [...] > :) Thanks for the comments, I'm still a little curious on information > though about the topic. And I hope you get them. I agree that getting more details on the specifics would be useful and interesting. But I'm still going to suggest that you always dispose classes that implement IDisposable. :) It's one thing for the authors of .NET, who have intimate knowledge with the inner workings of the run-time and the framework, to make a choice to not implement IDisposable even though a class might benefit from it. It's yet another for the rest of us to make decisions contrary to the accepted conventions exposed by .NET. You do so at your own peril. :) Pete On Apr 4, 10:28 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > I can't say whether you're missing something. I do know that a quick > Google search yielded this link:http://blogs.msdn.com/vancem/archive/2006/03/29/564854.aspx :) I did see that page before when searching "RWLS + dipose", it was the only page that showed up on the first google results page that had relavance. Aye, its not the ReaderWriterLockSlim class, but the comments at the bottom did give me suspecions that cleaning up RWLS's might not be needed that much. > In that particular example, you either need to decide whether you're going > to interrupt the threads that need the lock, or you're going to delay > cleanup of the lock until the threads are all done. You can't let the > threads run _and_ clean up the lock right away. Aye. I understand this design wise, but, in the case of the thread pool, you can't interrupt it. And waiting around for the threads to finish isn't always a perferred solution; maybe the locks were there to syncronize actions (IE in the case of a chat program, the thread could lock the send/recive method on a line basis, so if you stop locking the worst that could happen would be something like Msg1: "hello jim." Msg2: "test", resulting in "hetleslo jim .\nt". If your cleaning up, your probably not caring what else the threads going to do as long as it doesn't crash, etc. > This example stores the instance in a static variable, so IDisposable > wouldn't apply. Ahh. I always through using static on IDisposable objects was a no no. But yes, I see now that if the IDisposable object is living to the end of the application (which is the case of my RWLS's usally), making them static would simplify my code a lot, and might be the intended way of using them most the time. I'll look into this approach more at the very least :) Thanks for your comments, they were helpful. I usally use Monitor's (+lock statments) anyway, but wanted to try a few applications with RWLS and think static might be the way to go for some of the problems I had with them. NB > [...] >> In that particular example, you either need to decide whether you're [quoted text clipped - 5 lines] > Aye. I understand this design wise, but, in the case of the thread > pool, you can't interrupt it. Of course you can. Not that I was suggesting you use Thread.Abort(), but even that should work okay (exceptions are eaten by the thread pool, but your own code is aborted...the thread pool will just create a new thread later if needed). In reality, if you need a thread (thread pool or otherwise) to be interruptible, you just design it to be so. Use a volatile flag, have the code in the thread check the flag and exit when it can. Or any of a number of other signaling mechanisms that are available. > And waiting around for the threads to > finish isn't always a perferred solution; If not, then you need to design the thread to be interruptible. That's not a problem. > maybe the locks were there > to syncronize actions (IE in the case of a chat program, the thread > could lock the send/recive method on a line basis, so if you stop > locking the worst that could happen would be something like Msg1: > "hello jim." Msg2: "test", resulting in "hetleslo jim .\nt". If I were using a messaging application that produced a result like that, I would never ever run it again. At the very least, it suggests that the provider of that application was willing to let less-than-perfect behavior occur just to make their own life simpler. And worst, it's a strong clue that the provider may have underestimated the need to produce correct code. Synchronization exists to ensure correct, predictable behavior from the code. If the code isn't synchronized and is allowing output to be intermingled when that wasn't part of the user's intent, who knows what else may have been overlooked by the provider? > If your > cleaning up, your probably not caring what else the threads going to > do as long as it doesn't crash, etc. Well, first of all...the "as long as it doesn't crash" is an important consideration: how can you prove that your code won't crash if you've abandoned synchronization? Secondly, if you're cleaning up, then either it's okay to interrupt the threads or it's not. If it is, then you design those threads so that they can be interrupted and then you interrupt them. If it's not, then you have no choice but to wait for them to finish. What benefit is there in doing some of the cleanup early, and then still waiting on the threads? Especially when that cleanup removes objects that the threads need to use? Pete On Apr 5, 2:46 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > > [...] > >> In that particular example, you either need to decide whether you're [quoted text clipped - 52 lines] > > Pete I was talking about the Managed ThreadPool, not a custom made one; the one you usally access with "ThreadPool.QueueUserWorkItem(...)". After you start a thread on the threadpool then you can't stop it without a volatile flag. Maybe you could have the work items list themselves someplace static so you could call abort from another thread, but I'm not sure if its possible. > What benefit is there in doing some of the cleanup early, and > then still waiting on the threads? Especially when that cleanup removes > objects that the threads need to use? I can only list situations off the top of my head (which are quick examples). I'm sure people run into better (more relivant) situations naturally, but for example say you want to Restart (Application.Restart()) the application, or do something else (in the case of some kind of error, or special event), the user may very of well been the one to signal the "okay get on with it, I don't care what threads are doing I want to do something else now", then the user surly doesn't want to wait for any threads, so you clean up what you can really quick, set the violet flag to tell the threads to finish, and start the next task (letting the threads finish gracefully in the background). > If not, then you need to design the thread to be interruptible. That's > not a problem. Sometimes, for quick applications at least, re-designing your own ThreadPools, or such, to fit your situtation perfectly isn't acceptable (time wise). Instead they use the one already made and live by the limitations of that implementation, or work around them. NB > I was talking about the Managed ThreadPool, not a custom made one; So was I. > the > one you usally access with "ThreadPool.QueueUserWorkItem(...)". After > you start a thread on the threadpool then you can't stop it without a > volatile flag. You can call Thread.Abort() on one of those threads as easily as on any other thread, provided you have a reference to the thread. You can always get a reference to the thread by getting Thread.CurrentThread inside the worker thread. > Maybe you could have the work items list themselves > someplace static so you could call abort from another thread, but I'm > not sure if its possible. Yes, it's possible. And it's exactly what you'd have to do, should you decide to use Thread.Abort() to interrupt the thread. Personally, I feel that using Thread.Abort() is a bad idea, and it's not what I was suggesting you might do. But it certainly is possible. >> What benefit is there in doing some of the cleanup early, and >> then still waiting on the threads? Especially when that cleanup removes >> objects that the threads need to use? > > I can only list situations off the top of my head (which are quick > examples). Such as? Do you have an example of a scenario in which there's a benefit of doing some cleanup early, removing objects that the threads need to use, and then still waiting on the threads to complete? > I'm sure people run into better (more relivant) situations > naturally, but for example say you want to Restart [quoted text clipped - 6 lines] > and start the next task (letting the threads finish gracefully in the > background). But that's not a correct design, if the threads may want to use any of the data structures you are cleaning up before they complete. You could certainly design an application such that the entire context for a session is replaced when restarting, where none of the previous context is cleaned up until the threads are done. But then you're basically waiting for the threads to finish anyway, which is what I said you need to do. You simply cannot have valid code in which you clean up data structures that threads that continue to run may need to use. Your only choices are to wait for the threads to complete before cleaning up the data structures (either letting them run to completion or telling them to interrupt themselves), or to change the design so that those data structures are not needed by the threads. Any other design is flawed. >> If not, then you need to design the thread to be interruptible. That's >> not a problem. > > Sometimes, for quick applications at least, re-designing your own > ThreadPools, or such, to fit your situtation perfectly isn't > acceptable (time wise). Irrelevant. That's not what I'm suggesting anyone do. > Instead they use the one already made and > live by the limitations of that implementation, or work around them. I don't feel that a coder really has a choice with respect to writing correct code. No programmer deserving of that label has any business intentionally writing code that they know to be defective. Pete On Apr 5, 8:00 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > > I was talking about the Managed ThreadPool, not a custom made one; > [quoted text clipped - 77 lines] > > Pete Correct code is in the eye of the beholder. Just because your design has some limitations or work-arounds doesn't make it defective. > But that's not a correct design, if the threads may want to use any of the > data structures you are cleaning up before they complete. Who says the threads want to use any of the data structures you are clenaing up. Furthermore, there is a "disposed" at their disposal. if(!disposed) use structure. I always read that ThreadPool threads are lightweight; using them like suggested isn't correct (design wise as you say). If someone is using it like that then they probably should redesign the solution to not use the ThreadPool :P NB > Correct code is in the eye of the beholder. Just because your design > has some limitations or work-arounds doesn't make it defective. Just because some correct code has some limitations or work-arounds, that does not mean that all code with some limitations or work-arounds is not defective. >> But that's not a correct design, if the threads may want to use any of >> the >> data structures you are cleaning up before they complete. > > Who says the threads want to use any of the data structures you are > clenaing up. You did. You specifically said you are talking about cleaning up synchronization data structures that are used by the threads. > Furthermore, there is a "disposed" at their disposal. > if(!disposed) use structure. If they have the option of not using the structure, then they don't need the structure. In which case you should not have the structure at all. Conversely, if they need the structure, then you don't have the option of them not using the structure when it's been disposed. > I always read that ThreadPool threads are lightweight; using them like > suggested isn't correct (design wise as you say). If someone is using > it like that then they probably should redesign the solution to not > use the ThreadPool :P This has nothing at all to do with whether you are using the thread pool or some other mechanism. And I have no idea what you mean by "using them like suggested isn't correct". Using them as _who_ suggested? What suggestion is it that you feel isn't correct? Not that the answer to that question would have any relevance to the issue I'm writing about in this thread. But your paragraph is amibguous and I'm curious what you actually meant. Pete On Apr 6, 11:20 am, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > You did. You specifically said you are talking about cleaning up > synchronization data structures that are used by the threads. I think you miss read some place, I've never used the word data structures anywhere here, I said sychronizing actions with the chat program example if you thought that's the same thing. > If they have the option of not using the structure, then they don't need > the structure. In which case you should not have the structure at all. "They don't need the structure anymore" is what should be said, this does not mean they didn't need it before the event that signled the clean up. > Not that the answer to that question would have any relevance to the issue > I'm writing about in this thread. But your paragraph is amibguous and I'm > curious what you actually meant. > Any other design is flawed. I walk talking about this. To be more specific, basically I said that if I have a solution that cleans up the object immediatly, lets all the threads gracefully exit on their own, and doesn't affect the user (visually) any bit, then not only is this a correct design, but its more elegant in the respect of using the "disposed" variable as a violet flag (which, intuativaly, gets checked - and can immidatly exit the thread when it is checked and found violet). If the design forcably interrupts/aborts the threads then it can't be better than the situation above (IMO), and the above is just as correct as waiting for the threads, which could bring bad cases depending how long it takes to get between your violet flag checks, expecially when you don't have to be waiting around (performance wise). NB > On Apr 6, 11:20 am, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> > wrote: [quoted text clipped - 3 lines] > I think you miss read some place, I've never used the word data > structures anywhere here, You don't need to use the phrase "data structure" to be talking about a data structure. The ReaderWriterLockSlim class is a data structure. Rather than going through the rest of your post point by point though, I think the most productive thing is to focus on this: > [...] > I walk talking about this. To be more specific, basically I said that [quoted text clipped - 4 lines] > violet flag (which, intuativaly, gets checked - and can immidatly exit > the thread when it is checked and found violet). First of all, I think the word you want is "volatile". Secondly, what does that have to do with the paragraph I quoted in which you discussed the use of the thread pool? That said, let's ignore the non-sequitur for the moment and focus on what you did write. I note, by the way, that you have changed your suggestion so that your threads now exit when they note the synchronization object as being disposed, rather than simply continuing to process without the benefit of synchronization as you suggested earlier. For what it's worth, that doesn't actually change the reality of the situation though. You still need to wait for the threads to exit (or at a minimum, to signal back that they've acknowledged the "exit early" signal). In particular... Unless your Dispose() method is thread-safe (this is true for ReaderWriterLockSlim, but that class has no way to check whether it's actually disposed, so you must be talking about some other class), you cannot reliably check the disposed state as a way of knowing whether it's safe to use the synchronization object. For some other class to be thread-safe, you need to introduce some thread synchronization. Otherwise, one thread could be just about to dispose the synchronization object, but not yet have set the "disposed" flag, while another thread is about to try to use the object that's about to be disposed (having already checked the flag). If the latter thread now gets preempted and the first thread gets to run and dispose the object, the second thread is screwed when it eventually gets to run again. So, either you introduce a new volatile flag to indicate that the threads need to exit and not dispose your synchronization object until the threads have exited (in which case that's exactly the "signal the thread to exit" design I said was one legitimate way to address the issue), or you have to add a new synchronization object to the design to protect the retrieval of the dispose state (in which case, now you have to wait for all the threads to exit before you can dispose _that_ synchronization object). In other words, you have a paradox in your argument that always leads to creating a design that is exactly as I suggested it needs to be in the first place: you either need a signal to the threads to get them to stop processing, or you need to simply wait until they're completely done. Either way, you can't clean up until the threads are all done. If you like, feel free to post a concise-but-complete implementation that does what you think is okay, using a disposable synchronization object that your threads make use of as you've described above. I will be happy to point out in that concrete example the scenario in which your threads fail to produce correct behavior (whether because they simply don't produce synchronized output, or they actually crash trying to use a disposed object). Pete On Apr 6, 12:29 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > rather than simply continuing to process without the > benefit of synchronization as you suggested earlier. I don't remember ever saying that. If you thought that I'm sorry, but, as a said, if your checking the disposed flag before using a disposable object, of course your gonna have an else statment to stop/ clean up if your object is disposed... > The ReaderWriterLockSlim class is a data structure. If thats the structure your talking about then the "If your thread wants to use the structure" statment you said is invalid... The thread doesn't want to use that structure anymore if it's disposed, else you wouldn't be disposing. > Secondly, what > does that have to do with the paragraph I quoted in which you discussed > the use of the thread pool? ... The part about designing the solution to interrupt the thread. Specifically if your thread pool is designed to have your worker threads list themselves in a static variable, thus points to the "using them like suggested isn't correct", and your statment about having to interrupt the thread pool, there is no other way. I was pointing out the valid alternative to "design is flawed". > So, either you introduce a new volatile flag to [...] I never said replace the violate flag, its like batman and robin. If the violate flag is batman, then the dispose would be robin. You'd still need a way to tell the threads to stop without disposing your object. Your race condition situation is true for anyone trying to use the disposed flag in a multithreaded situation. You might as well be saying that the dispose pattern as we know it is only made for STA applications. IE Since, is most online examples of the dispose pattern, the disposed flag gets set after you clean up the managed resources. What if you call Dispose on an object, then your other thread checks the disposed flag and enters the if statment, then the disposed method sets the dispose to false. Application will crash. Will it ever happen? Most likly never, can it happen? Yes. It's how multithreaded programers live. I have to say, this conversation is getting a little pathedic... Definally off topic... I thought it'd be fun to make a "concrete" example :) Please tell me the incorrect sychronization, or crash. Only requirment is that when "Hello" gets displayed, "Goodbye" 100% has to be displayed. If it doesn't then my program failed. Feel free to add Sleeps anywhere. It uses a disposable sychronization object (RWLS). class Program { static void Main(string[] args) { Class1 myTest = new Class1(); myTest.StartWork(); myTest.Dispose(); Thread.Sleep(1000); } } class Class1 { private ReaderWriterLockSlim myStringLock = new ReaderWriterLockSlim(); public void StartWork() { ThreadPool.QueueUserWorkItem(new WaitCallback(test)); ThreadPool.QueueUserWorkItem(new WaitCallback(test)); } public void test(Object nullState) { if(!disposed) myStringLock.EnterWriteLock(); else return; try { foreach(string A in new string[] { "hello", "goodbye" }) Console.WriteLine(A); } finally { if(!disposed) myStringLock.ExitWriteLock(); } } private bool disposed = false; public void Dispose() { Dispose(true); } protected virtual void Dispose(bool disposing) { if(!disposed) { disposed = true; if(disposing) myStringLock.Dispose(); } } } > I thought it'd be fun to make a "concrete" example :) Please tell me > the incorrect sychronization, or crash. Only requirment is that when > "Hello" gets displayed, "Goodbye" 100% has to be displayed. If it > doesn't then my program failed. Feel free to add Sleeps anywhere. It > uses a disposable sychronization object (RWLS). Sure, no problem. See code posted below. The important change is that I added two events to ensure that the two threads were pre-empted in precisely the order that would cause a problem were the thread scheduler to happen to randomly schedule the threads in that order. I also removed the second thread pool worker. It's not necessary or relevant. It's very important that you understand that there's nothing magic about the events I added. They don't do anything except ensure broken behavior _every_ time. The exact order they impose on the code is an order that the thread scheduler could at any time wind up causing to happen all by itself. Without proper synchronization, there's no way to ensure that it doesn't. Enjoy. :) I hope that having a concrete, broken example to look at helps illustrate better the risk in the design you've proposed. Pete using System; using System.Collections.Generic; using System.Linq; using System.Text; using System.Threading; namespace TestBadSynch { class Program { static void Main(string[] args) { Class1 myTest = new Class1(); myTest.StartWork(); myTest.Dispose(); Thread.Sleep(1000); } } class Class1 { private ReaderWriterLockSlim myStringLock = new ReaderWriterLockSlim(); private AutoResetEvent _are1 = new AutoResetEvent(false); private AutoResetEvent _are2 = new AutoResetEvent(false); public void StartWork() { ThreadPool.QueueUserWorkItem(new WaitCallback(test)); // ThreadPool.QueueUserWorkItem(new WaitCallback(test)); } public void test(Object nullState) { if (!disposed) { _are2.Set(); _are1.WaitOne(); myStringLock.EnterWriteLock(); } else { return; } try { foreach (string A in new string[] { "hello", "goodbye" }) Console.WriteLine(A); } finally { if (!disposed) { myStringLock.ExitWriteLock(); } } } private bool disposed = false; public void Dispose() { Dispose(true); } protected virtual void Dispose(bool disposing) { _are2.WaitOne(); if (!disposed) { disposed = true; if (disposing) myStringLock.Dispose(); _are1.Set(); } } } } > On Apr 6, 12:29 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> > wrote: >> rather than simply continuing to process without the >> benefit of synchronization as you suggested earlier. > > I don't remember ever saying that. You wrote: waiting around for the threads to finish isn't always a perferred solution; maybe the locks were there to syncronize actions (IE in the case of a chat program, the thread could lock the send/recive method on a line basis, so if you stop locking the worst that could happen would be something like Msg1: "hello jim." Msg2: "test", resulting in "hetleslo jim .\nt". If your cleaning up, your probably not caring what else the threads going to do as long as it doesn't crash, etc From the statement "waiting around for the threads to finish isn't always a perferred [sic] solution", we know that you intend for the threads to continue processing. The statement "if you stop locking the worst that could happen would be..." further implies that it's not that the threads will stop executing when they notice the lock's not around, but rather that they will simply continue working without synchronization. > If you thought that I'm sorry, I only thought you said that because you did. I'm happy if you want to retract the previous example, but you did post that scenario as an example. > but, as a said, if your checking the disposed flag before using a > disposable object, of course your gonna have an else statment to stop/ > clean up if your object is disposed... You can't check the disposed flag without synchronizing access to it. >> The ReaderWriterLockSlim class is a data structure. > > If thats the structure your talking about then the "If your thread > wants to use the structure" statment you said is invalid... The > thread doesn't want to use that structure anymore if it's disposed, > else you wouldn't be disposing. You can write "the thread doesn't want to use that structure anymore" all you want. Until you've actually _shown_ why it's valid for the thread to "not want that structure anymore", it's meaningless. If the thread needed the structure before, no other thread can safely dispose the structure until the thread has in some way signaled that it no longer needs it. You can't just dispose it and assume that the other thread can detect that before it next tries to use it. >> Secondly, what >> does that have to do with the paragraph I quoted in which you discussed [quoted text clipped - 6 lines] > having to interrupt the thread pool, there is no other way. I was > pointing out the valid alternative to "design is flawed". Again: I never once suggested that you _should_ be aborting thread pool threads. Please stop using that red herring. It has nothing to do with this. >> So, either you introduce a new volatile flag to [...] > [quoted text clipped - 7 lines] > saying that the dispose pattern as we know it is only made for STA > applications. Well, many disposable objects are specifically documented as not being thread-safe. However, Microsoft does also specifically _recommend_ that if you implement Dispose() that it be valid to call Dispose() multiple times safely. But most disposable classes don't provide any mechanism for checking to see whether they are disposed. Only a class that does, _and_ which is documented as being thread-safe, would permit the sort of design you're talking about. I'm not aware of any such class in .NET. Certainly the classes we've discussed in this thread don't fit that description. > IE Since, is most online examples of the dispose > pattern, the disposed flag gets set after you clean up the managed > resources. None of those examples are intended to show a thread-safe implementation of Dispose(). They simply use the flag as an optimization. > What if you call Dispose on an object, then your other > thread checks the disposed flag and enters the if statment, then the > disposed method sets the dispose to false. Application will crash. > Will it ever happen? Most likly never, can it happen? Yes. It's how > multithreaded programers live. NO! That's not how competent programmers who write multi-threaded code live. "Most likely never happen" is NEVER an appropriate design philosophy. For classes that follow Microsoft's recommendation that the Dispose() method be safe to call multiple times from multiple threads, you can get away with doing so. But only for those classes, and only because the Dispose() method has already been designed to support that. There's no question of whether that will ever break for those questions; it won't, because the class has been designed to support that. But beyond that, being able to call Dispose() multiple times isn't the same as whether it's safe to dispose an object in one thread while another thread might try to use it. The two are related issues, but not the same at all. > I have to say, this conversation is getting a little pathedic... > Definally off topic... It's no more off-topic than any other programming design thread, of which we have many here in this newsgroup. If anything, I find that this discussion is of great importance, because if anyone follows your advice, they are heading down the road to serious trouble. I want to make sure that it's as clear as possible that you cannot design multi-threaded code in the way you suggest and still have correct code. Even better would be if I'm able to convince you, but I recognize that not all people are prepared to admit that they have made a mistake. Pete > I only thought you said that because you did. I'm happy if you want to > retract the previous example, but you did post that scenario as an example. That was an example to illustrate what would happen if you got past the (disposable) lock before it disposed, but not result in a crash and be a valid "not care about the results anymore"; user wise. If it was before the lock when it disposed, then it wouldn't result in any output because it'd see the object is disposed and just skip to the end. It's like using it to generlize the topic to chat programs only - "whats all this talk about thread pools, I thought we were talking about chat programs" > Again: I never once suggested that you _should_ be aborting thread pool Amusing, red herring? You gave the example. Said its the only valid way if you were in that situation to safly clean up (which I dissagreeded with). You can keep following the straw man argument above if you like, but don't tell me to not try to refret the "only" way to safly dispose the thread pool without waiting argument you gave, and then call it a red herring. > However, Microsoft does also specifically _recommend_ that > if you implement Dispose() that it be valid to call Dispose() multiple > times safely. I didn't design the dispose pattern. It comes with a "disposed" flag that they expect you to use. What if 2 objects try to dispose at the same time, and both objects try setting the "disposed" flag to false at the same time? Is the design patter flawed? Not really, the situation doesn't really occur ever, but it probably can since who knows when the GC will call it. I wouldn't spend too much time trying to fix this situation even though it can potentially exsist. > "Most likely never happen" is NEVER an appropriate design > philosophy. I'm not too sure about this statment... It's not that perfect of a world. > It's no more off-topic than any other programming design thread :) It's definally off-topic, but I retract my previous statment. It's not pathedic, I don't know why I said that. I don't see myself as the kind of person who has a problem admiting I made a mistake, I just don't agree with your view. In my opinion, it's possible to (as reliably as other programs), clean up an object and not be forced to wait for the threads inside it to finish, or force threads to stop with an abort. Mainly in the case where you don't care about the result of the threads anymore. Another example could be that you send 100 threads off to get an answer, you got the answer when the 3rd thread returned. You can do clean up and ignore the other threads as long as you have dispose checks (or try catch statments (or both)), around the objects you clean up. Now don't get me wrong. A disposable sychronization lock did seem very strange to me (point of the topic), and obviously makes using this type of lock in a situation where you want to do fast clean up makes it harder (if statmtnet, or try catch statments, everytime you try and aquire a lock). But this isn't a bad design, the benifites can be good. IE in the previous example changing all the "if(!disposed) myStringLock.EnterWriteLock(); else return;" to "try { myStringLock.EnterWriteLock(); } catch { return; }" would fix any possible crashes, and be a concret example with no possible chance of crashing, yet have everything synched. :) NB hehe, We got a synch problem going on! :) Aye I seen the case where 1. if(!disposed) 2. disposed = true; 2. RWLS.dispose(); 1. RWLS.Aquire Lock How often? 0.0001% maybe. As listed in previous example try this one class Program { static void Main(string[] args) { Class1 myTest = new Class1(); myTest.StartWork(); myTest.Dispose(); Thread.Sleep(1000); } } class Class1 { private ReaderWriterLockSlim myStringLock = new ReaderWriterLockSlim(); public void StartWork() { ThreadPool.QueueUserWorkItem(new WaitCallback(test)); ThreadPool.QueueUserWorkItem(new WaitCallback(test)); ThreadPool.QueueUserWorkItem(new WaitCallback(test)); ThreadPool.QueueUserWorkItem(new WaitCallback(test)); } public void test(Object nullState) { try {myStringLock.EnterWriteLock();} catch(Exception) { return; } foreach(string A in new string[] { "hello", "goodbye" }) Console.WriteLine(A); try {myStringLock.ExitWriteLock(); } catch(Exception) { return; } } private bool disposed = false; public void Dispose() { Dispose(true); } protected virtual void Dispose(bool disposing) { if(!disposed) { disposed = true; if(disposing) myStringLock.Dispose(); } } I added 2 more queue work items because I wanted too :P I don't think this example can crash. Putting both the !disposed would make ~99.999999% of the users never trigger the catch statment, makes one almost not want to put it there ;) NB > hehe, We got a synch problem going on! :) Aye I seen the case where > [quoted text clipped - 4 lines] > > How often? 0.0001% maybe. As listed in previous example try this one Your new example fails to crash solely by virtue of catching the exception. That doesn't mean it's thread-safe; it just means you wrapped bad code in an exception handler and decided to ignore the design problem. I can write any program so that it doesn't crash, using the same technique. If using that technique were a valid proof of correct design, then no program could be considered incorrect. Pete > [...] >> However, Microsoft does also specifically _recommend_ that [quoted text clipped - 3 lines] > I didn't design the dispose pattern. It comes with a "disposed" flag > that they expect you to use. Not for the purpose of making the call thread-safe with other operations on the class, they don't. > What if 2 objects try to dispose at the > same time, and both objects try setting the "disposed" flag to false > at the same time? If it's safe to do whatever work inside the method from two different threads at the same time, then there's no problem at all. All that happens is some redundant work. There's no risk at all in having two different threads setting the same flag to "true" at the same time. Or even "false" as you wrote, if you really think that setting the "disposed" flag to "false" in the "Dispose()" method is the right way to do it. > Is the design patter flawed? I never said it was flawed. But it's not thread-safe with other operations on the class. > Not really, the > situation doesn't really occur ever, but it probably can since who > knows when the GC will call it. I wouldn't spend too much time trying > to fix this situation even though it can potentially exsist. I hope no one is paying you for your coding efforts then. It's one thing to be ignorant of a design problem and overlook it. It's quite another to know it exists and just ignore it. The latter is professionally irresponsible. > [...] > In my opinion, it's possible to (as reliably as other programs), clean > up an object and not be forced to wait for the threads inside it to > finish, or force threads to stop with an abort. If it were a matter of opinion, then your opinion might matter. But it's not a matter of opinion. You can't write reliable code in that way. > Mainly in the case > where you don't care about the result of the threads anymore. Whether you care about the result of the threads anymore is irrelevant. Cleaning up something the threads need for proper operation can result in data corruption or an immediate crash. Neither is appropriate in a program that is to be considered reliable. The only reason you can ignore this issue is if you are happy with the program being unreliable. Personally, I don't write programs that I know to be unreliable. > Another > example could be that you send 100 threads off to get an answer, you > got the answer when the 3rd thread returned. You can do clean up and > ignore the other threads as long as you have dispose checks (or try > catch statments (or both)), around the objects you clean up. Not and have a reliable program, you can't. And it's so easy to write the code reliably, that it's just silly to not do so. You have spent more time arguing about the issue than it should take you to implement a "signal, wait, clean up" design in half a dozen programs. > [...] > IE in the previous example changing all the "if(!disposed) > myStringLock.EnterWriteLock(); else return;" to "try > { myStringLock.EnterWriteLock(); } catch { return; }" would fix any > possible crashes, and be a concret example with no possible chance of > crashing, yet have everything synched. As I noted in my other post, wrapping the call in a try/catch doesn't address the underlying design issue, nor does it any way validate the design choice. That said, I find it amusing that you don't like the idea of aborting a thread (as well you shouldn't), but you're perfectly happy to have it throw an exception that you catch and use to terminate the thread. There's very little difference between the two approaches. Pete On Apr 6, 7:53 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > As I noted in my other post, wrapping the call in a try/catch doesn't > address the underlying design issue, nor does it any way validate the > design choice. That said, I find it amusing that you don't like the idea > of aborting a thread (as well you shouldn't), but you're perfectly happy > to have it throw an exception that you catch and use to terminate the > thread. There's very little difference between the two approaches. Amusing, are there any other requirments? Am I allowed to use if statments? :) Its a joke :P. You say "it doesn't crash", but thats not just the case, you should be saying "it doesn't crash, and maintains the strick sychronization requirments", and all other requirments/functions of the program for that matter, with no possibility of ill effects. It does exactly what the client paid for, and doesn't have any dead weights running around. I don't know why you wouldn't call it a valid design, it uses a boolean to check the object before using it, and in the rare case of object being disposed between the ")", then it does whatever cleanup the thread might want to do, and exit, in a valid way to maintain sychronization. From what your saying here it's like you've never opened a file in your life as a programmer (which is the same design pattern as I'm using). You check the file to see if its valid, and then open it. Can something grab the file between the check and opening? Definally, does it make it a bad design? Definally not. You show me a method of opening a file without a try/catch statment that doesn't crash, and I'll show you how to check for use of a disposable object that can be disposed anytime. Rest of your statments are just plaintly close minded. Telling people there is no way to do something (correctly), is extremly ignorant, "I hope no one is paying you for your coding efforts then" with that kind of mindset, a real programmer see's a problem, and accoplish's it. He doesn't whine about a try/catch statment, he whines about the performance/robustness/correctness (problem wise) of a solution. NB PS: I'm not saying this is the only way to do things. I've made programes with all three techniques mentioned here and will agree that signling threads to finish and waiting for them to finish to dispose is one way to do it. It doesn't make it the best way for all cases in the world... unlike your mindset, the world is not that small of a place. > [...] > Amusing, are there any other requirments? Am I allowed to use if [quoted text clipped - 4 lines] > possibility of ill effects. It does exactly what the client paid for, > and doesn't have any dead weights running around. Exceptions are costly, and so it's certainly not true that your design "doesn't have any dead weights running around". As far as the question of "no possibility of ill effects", in your specific example that's true, but you can't extrapolate that to any arbitary implementation that uses the same technique. > I don't know why you wouldn't call it a valid design, it uses a > boolean to check the object before using it, and in the rare case of > object being disposed between the ")", then it does whatever cleanup > the thread might want to do, and exit, in a valid way to maintain > sychronization. Why do you bother checking the boolean at all? It's not protecting you in any way. It's a bad design because it has code that tries to do something, but because you haven't designed the code to be reliable, you wind up having to add a bandage to catch the fact that the code isn't reliable. This code performs its duty perfectly: try { string[] rgstr = { "one", "two", "three" }; for (int i = 0; i <= rgstr.Length; i++) { Console.WriteLine(rgstr[i]); } } catch { } But that doesn't mean that it's been designed correctly. And in fact, it hasn't been designed correctly. The above is just like your proposed "catch disposed exception" solution, which is just as incorrect a design. > From what your saying here it's like you've never opened a file in > your life as a programmer (which is the same design pattern as I'm > using). Opening a file and catching a possible failure is nothing like the design pattern you're using. Just because both examples use try/catch, that does not make them the same design pattern. Still, the "open file" pattern you describe is just as flawed as the "catch disposed exception" pattern you're promoting. Specifically... > You check the file to see if its valid, and then open it. > Can something grab the file between the check and opening? Definally, > does it make it a bad design? Definally not. Writing code that checks a file to see if it's valid and then tries to open it is stupid. Because of the exact issue you're describing, there's absolutely no point in doing any verification of the file before opening it. You should just try to open it. That's your verification right there. > You show me a method of opening a file without a try/catch statment > that doesn't crash, and I'll show you how to check for use of a > disposable object that can be disposed anytime. The two situations are entirely different. It's trivial to write code that doesn't need to catch an "object disposed" exception. But as you pointed out, because the file system is not under the process's control, there's no way to ensure that opening a file won't throw an exception. > Rest of your statments are just plaintly close minded. Telling people > there is no way to do something (correctly), is extremly ignorant, You're welcome to your opinion. But I'm welcome to mine too, and I certainly am of the opinion that it's extremely ignorant to write code that throws exceptions when you could just as easily write it correctly and without the code failing intentionally. > "I hope no one is paying you for your coding efforts then" with that kind > of mindset, a real programmer see's a problem, and accoplish's it. No. A real programmer doesn't just make his program work. He designs it with care and attention to detail, making it maintainable, robust, and efficient. He does not just add duct tape until the thing works. He strives to make the solution as elegant as it is useful. Pete > Exceptions are costly > Why do you bother checking the boolean at all? Amusing. You answered your own quesiton, I'm just unsure if you realized it yourself? Exceptions are costly, you want to minimize them. > This code performs its duty perfectly: Other than the fact that your not catching any specific excpetion I can see. If you wanted a global exception handler there are better ways. > But that doesn't mean that it's been designed correctly. It does... Putting a try/catch statment around potentially hazardous code is exactly what it was designed to do. A good programmer can see a hazard (even 0.001% case), and fix it with the try statment - acuratly so that program continues normally as intended. > You should just try to open it. That's your verification right there. If anyone is readining (which probably not anymore), I'd highly suggest not listening to this part here. Minimize your excpetions because they are costly. For example. If pete wants to open a file he's not 100% sure exsists, he'll "try( File.Open(file) }..." while what you really should do is a "if(File.Exsists(file)) open" and wrap that in a try statment if needed for a chance it doesn't exsits after you just checked it. Unless your very sure the file exsists, you should do the Exsists check to maxmize performance. End result, minimize your exceptions people. It's simple math, heres the equation: Performance Overhead (ms) = AverageChanceOfExcpetion% * ExceptionCost + AverageChanceOfNoneException% * CheckCosts AKA CheckCosts = ~0.1ms. ExceptionCosts = 1000x that. What you want to minimize? > He does not just add duct tape until the thing works. End result, the program, binary wise, would be able to stand up to any other implementation in any kind of benchmarking, performance, reliablity tests, not only that, it may even surpass the other designs. What would the judges(clients) say is better? They don't care if if one uses a string and the other uses a char[], they care what gets the job done the fastest. NB > [...] >> You should just try to open it. That's your verification right there. > > If anyone is readining (which probably not anymore), I'd highly > suggest not listening to this part here. I would be surprised if there is anyone still following this thread who agrees with the design philosophy you're promoting, including your suggestion that checking for validity of a file operation before actually attempting the operation is a worthwhile thing to do. > [...] > End result, the program, binary wise, would be able to stand up to any [quoted text clipped - 3 lines] > care if if one uses a string and the other uses a char[], they care > what gets the job done the fastest. Performance is one the last criterias that should be of concern in program design. Maintainability, simplicity, and reliability are much more important. And for certain, writing bad code just because you think it will perform better is just plain wrong. Pete On Apr 7, 8:00 pm, "Peter Duniho" <NpOeStPe...@nnowslpianmk.com> wrote: > > [...] > >> You should just try to open it. That's your verification right there. [quoted text clipped - 21 lines] > > Pete You remind me of steve jobs. The crazy crazy personality type who doesn't care about money, he just wants to make sure everyone does things his way. In comparason I'd be like Bill Gates, still crazy, but money crazy. Incase the analagy is lost on you, Money = Perfomrnace, and Design is you freaking out about people doing things your way or they are wrong. It's also like the Nazi's "Theres no place in this world for a disposable thread sychronization object, GAS THEM". At least your not saying it wouldn't run worst :P NB > You remind me of steve jobs. The crazy crazy personality type who > doesn't care about money, he just wants to make sure everyone does > things his way. Neither of us know Jobs personally and thus are not qualified to make any sort of determination regarding his personality. That said, I doubt it's true that he doesn't care about money. > In comparason I'd be like Bill Gates, still crazy, > but money crazy. I have somewhat better insight regarding Gates, though I still wouldn't say I know him personally. I would definitely not describe him as "money crazy". But regardless... > Incase the analagy is lost on you, Money = Perfomrnace, and Design is > you freaking out about people doing things your way or they are > wrong. You have some learning to do with respect to the economics software development. It is not true that "Money = Performance". In fact, your proposed design has very minimal impact on performance, certainly not enough for any client to notice, and yet it has a significant negative impact on development cost. Assuming the client will pay a given amount of money for a solution, the less time you spend on making that solution, the more profit you have. Even a person who is "money crazy" would be a fool to choose the design you've proposed. That design is only going to cost them money. There's no way it's going to improve their bottom line. > It's also like the Nazi's "Theres no place in this world for a > disposable thread sychronization object, GAS THEM". Really? You really think a Nazi analogy is appropriate here? Are you intentionally invoking Godwin's Law? Sheesh. And you can't even be bothered to do it in a relevant way, seeing as how nothing I've written has anything to do with whether there's a "place in this world for a disposable thread synchronization object". Pete I got a relation to Mac and Nazi in one post, your looking evil now, wuahah. > It is not true that "Money = Performance". In a metaphore or analogy, it can mean anything I want as long as the persons smart enough to pass those classic IQ tests (_ is to _ as _ is to _). Else I have to spoon feed it to them... which isn't as fun. People think in different ways than others. Forcing someone to write code in a certain way will only prologe the development process. If a person wants to dispose straight away and let threads do their thing, then thats just the way he thinks. For small classes it might even be the best way IMO. I'd like to see your proof on this method taking longer/cost more to develop, but I think we both know there isn't any (expecially if the developer already has that solution in his mind... you want to force a different thought process into this guys head with no "good" reasion as I can see). You agree performance wise it isn't worst, you seem to think it'll be harder to maintain which isn't true either - as long as its commented right it'll be just as easy as any other implementation. You seem to think it'll take longer to develop, for some maybe, the reverse is true for others. Now if you were giving valid reasion's, even I'd convert and say "oww definally want to do it that way, I see why now", but the "nope, the design is flawed because it's not the way I do it. You way works just as good, but costs more to develop." is a very hard argument stance to succeed in. NB > I got a relation to Mac and Nazi in one post, your looking evil now, > wuahah. > >> It is not true that "Money = Performance". > > In a metaphore or analogy, I know. But it's not a correct one. That's my point. Had you bothered to read the rest of what I wrote, you'd understand what I meant. > [...] > People think in different ways than others. Forcing someone to write > code in a certain way will only prologe the development process. It is objectively true that the design I've suggested is simpler to implement and simpler to maintain. For an inexperienced programmer it may well be that it might take them a little more time to learn the concept, but in the long run even they will spend less time on it. > If a > person wants to dispose straight away and let threads do their thing, > then thats just the way he thinks. Computers only think one way. It is the job of the programmer to learn to think the way the computer thinks, so that he is not fighting the computer as he writes the code. Beyond that, while it is not possible to write correct code that disposes objects threads might use before those threads exit, it's certainly possible to write "fire and forget" code that allows the clean-up to happen while the rest of the application continues on doing whatever it was doing. This should be sufficiently close to the paradigm of "dispose straight away and let threads do their thing" that even the person for whom "that's just the way he thinks" could easily comprehend and use such a design. > For small classes it might even be > the best way IMO. I'd like to see your proof on this method taking > longer/cost more to develop, but I think we both know there isn't any I guess that depends on what you consider proof. However, your proposed design can be implemented in a couple of ways: a single try/catch at the very top of the thread's call chain, or a try/catch around each and every possible use of a disposable object. In the former case, the initial implementation is quick, but then you create a situation in which you can't tell whether you're catching an expected exception or one that represents some other bug. That's a situation that is demonstrably a huge maintenance risk. In the latter case, you can ensure that you're not catching exceptions that shouldn't be hidden. But at the very least you've now committed to a huge amount of work to wrap every single one of your uses of a disposable object in a try/catch handler. In addition, you now introduce the risk of missing one of those uses. These are clear code maintenance problems, ones that will necessarily and unavoidable result in higher maintenance costs for the code. On the other hand, the design I've proposed carries no hidden performance costs, is 100% predictable, and as a result of that can be easily shown to correctly deal with tearing down worker threads. > (expecially if the developer already has that solution in his mind... > you want to force a different thought process into this guys head with > no "good" reasion as I can see). You agree performance wise it isn't > worst, you seem to think it'll be harder to maintain which isn't true > either I don't "seem to think". I know. > - as long as its commented right it'll be just as easy as any > other implementation. No, it won't. Because of the issues I describe above, the exception-based design inherently carries higher maintenance costs. > You seem to think it'll take longer to develop, > for some maybe, the reverse is true for others. I never said it would necessarily take longer to develop. In fact, if you just wrap the whole thread in a single try/catch, the initial development cost is almost certainly lower with your proposed design. But that's not the only cost that is of concern, and in fact it's the least important cost to consider. The total cost of maintaining the code over the lifetime of the code is what you need to consider. > Now if you were giving valid reasion's, even I'd convert and say "oww > definally want to do it that way, I see why now", but the "nope, the > design is flawed because it's not the way I do it. I have provided numerous reasons, all of them valid. It's unfortunate that you are so committed to your previous position that you cannot see the validity of what I've written. But it doesn't change the facts. By the way, much of what I've said is based on my own personal experience over the years. However, there's a very good book by Steve McConnell, called "Code Complete", that does a good job of covering these kinds of issues. I don't recall off the top of my head whether it addresses this specific kind of design issue, but a person who reads the book and takes to heart the general coding philosophy that McConnell is promoting will naturally understand what I'm talking about here. > You way works just > as good, but costs more to develop." is a very hard argument stance to > succeed in. To the contrary. I've made very specific statements about where and why the design I propose is simpler and lower-cost to maintain over the long run than the design you propose. You have ignored those statements wholesale, rejecting them for reasons known only to you. But that doesn't mean they are invalid. Pete > In the former case, the initial implementation is quick, but then you > create a situation in which you can't tell whether you're catching an > expected exception or one that represents some other bug. Your catching a very specific exception "Object null exception", other exceptions wont be caught. > But at the very least you've now committed to a > huge amount of work to wrap every single one of your uses of a disposable > object in a try/catch handler. It's a lock object... you grab it once and release it shortly after, probably 75%+ of the time. Furthermore, if you implement it this way, then users have a lot easier time using the object. They simply call ".Dispose()" when they are done with it (little more effort in the design I agree to ensure proper clean up in all situations). The other solutions put more effort in different parts of the project (IE clean up methods, thread stoping. Putting logic like this in the dispose would world very well), which basically pushes the responsablity onto the user of the class because you don't want to harder maintance. In my eyes, maintainence is as simple, it intuativly becomes 'thread safe', and easier for others to use. Weighed against your "harder to maintain even with good commenting". It's not a replacment, they both have different pros and cons. Surly you should be able to see that... NB >> In the former case, the initial implementation is quick, but then you >> create a situation in which you can't tell whether you're catching an >> expected exception or one that represents some other bug. > > Your catching a very specific exception "Object null exception", other > exceptions wont be caught. First of all, it's the ObjectDisposedException. Second, so what? You are making the assumption that you will never write any code that accidently uses an object that's already disposed. That's a rather broad assumption, one that will easily lead to higher maintenance costs if and when it turns out to be false. >> But at the very least you've now committed to a >> huge amount of work to wrap every single one of your uses of a [quoted text clipped - 3 lines] > It's a lock object... you grab it once and release it shortly after, > probably 75%+ of the time. First of all, it's not just a lock object. It's any object that you want to clean up. Or are you saying that you have this preference to dispose an object that the threads are using only when it's a synchronization object, and that you will wait to dispose anything else the threads are using until they are done? Unless you're saying that, then the text you quoted above applies to _every_ use of _every_ disposable object. Secondly, there's nothing about your proposed design philosophy that stated you'd apply it only in very simple scenarios. Synchronization objects are often used in multiple places, and there's really no theoretical limit to the number of places they could be used. It depends only on the overall complexity of the code. With the design I've proposed, no matter how complex the code in the thread gets, you still have a very simple strategy for exiting the thread. With your proposal, the exit strategy for the thread increases in complexity as the complexity of the rest of the thread code does as well. Again, higher maintenance costs. > Furthermore, if you implement it this way, then users have a lot > easier time using the object. They simply call ".Dispose()" when they > are done with it (little more effort in the design I agree to ensure > proper clean up in all situations). There's absolutely no reason at all that, if that's of a concern to you, the object cannot deal with the "fire and forget" disposal using the design I've proposed. Nothing about the design forces the disposal to be pushed onto the client of the class. > The other solutions put more > effort in different parts of the project (IE clean up methods, thread > stoping. Putting logic like this in the dispose would world very > well), which basically pushes the responsablity onto the user of the > class because you don't want to harder maintance. Wrong. See above. > In my eyes, maintainence is as simple, it intuativly becomes 'thread > safe', and easier for others to use. First, as I point out above, ease of use need not be sacrificed. But beyond that, ease of use is merely one part of maintainability. Your proposed design is inherently more complicated and requires more code to implement. These drastically increase maintenance costs. > Weighed against your "harder to > maintain even with good commenting". It's not a replacment, they both > have different pros and cons. Surly you should be able to see that... There's nothing to see. Your proposal is inherently flawed and is not necessary in order to achieve any of your stated goals. It's a lose-lose situation. Pete > That's a rather broad assumption, > one that will easily lead to higher maintenance costs if and when it turns > out to be false. Not really, the try statment is designed to deal with that exact exception. As a result it'll clean up as intended even if it wasn't the dispose method that made it null. > Or are you saying that you have this preference to dispose > an object that the threads are using only when it's a synchronization > object, I was still talking about the example provided. It only has the sychronization object. Yes all "fast clean" dispose objects need to be checked before use, but in practice this usally doesn't result in many objects (~0-1). Can I make a class that uses 100 disposable objects? Yes. Can I use a bazuka to kill an ant? Yes. > Secondly, there's nothing about your proposed design philosophy that > stated you'd apply it only in very simple scenarios. It doesn't have to. It depends entirly on the solution. If the amount of sychronization is simple though this pattern shines most. Sychronization could be simple even in huge objects as well. > Nothing about the design forces the disposal to be > pushed onto the client of the class. If your doing automatic clean up (IE in the dispose method), its not going to work out very well, unless your simply killing the threads. Anyway you do it, it'll either put more effort into your design (effort that isn't needed in the other design since this case doesn't apply / or already implemented), or your pushing it onto the client of the class to make sure the object can be disposed before they dispose. > Your > proposed design is inherently more complicated and requires more code to > implement. A couple try/catch statment with some simple clean up in the catch portion, vs whatever methods your creating (or automatic cleanup logic). They probably equal out, the only difference is one pushes it into the code the other localizes it. NB >> That's a rather broad assumption, >> one that will easily lead to higher maintenance costs if and when it [quoted text clipped - 4 lines] > exception. As a result it'll clean up as intended even if it wasn't > the dispose method that made it null. You don't seem to be paying attention to what I wrote. The point is that you may accidently use an object that was disposed before the program was done with the thread. The thread won't "clean up as intended". It'll clean up before intended. >> Or are you saying that you have this preference to dispose >> an object that the threads are using only when it's a synchronization >> object, > > I was still talking about the example provided. So you're not talking about a general design pattern at all then. I'll grant: with the very specific example you provided (that is, the demo code you posted) it really doesn't matter _what_ the implementation is. The code's too short for it to make a difference. I'm talking about real code. You know, the kind that does actual work. > It only has the > sychronization object. Yes all "fast clean" dispose objects need to > be checked before use, but in practice this usally doesn't result in > many objects (~0-1). Can I make a class that uses 100 disposable > objects? Yes. Can I use a bazuka to kill an ant? Yes. Practically any interesting code is going to use disposable objects, and in the general case it could easily be a large number of disposable objects. With your proposal, assuming you're not limiting it just to the degenerate case you posted, your suggestion requires a try/catch around literally every use of every disposable object (assuming you want to avoid the first problem I described, in which you catch and suppress unintended uses of disposed objects). >> Secondly, there's nothing about your proposed design philosophy that >> stated you'd apply it only in very simple scenarios. > > It doesn't have to. It does if you want to maintain it as a workable design philosophy (and even then, it's still higher maintenance than the correct way to do it). > It depends entirly on the solution. If the > amount of sychronization is simple though this pattern shines most. "This pattern" doesn't "shine" at all. It's bad even in the simple scenario where you can at least look at all the code and not worry about it breaking. And in a real-world scenario, it's a massive maintenance headache. > Sychronization could be simple even in huge objects as well. We're not talking about synchronization. We're talking about object disposal. >> Nothing about the design forces the disposal to be >> pushed onto the client of the class. > > If your doing automatic clean up (IE in the dispose method), its not > going to work out very well, unless your simply killing the threads. Again, you don't seem to be paying attention. My proposed design lends itself very well to "automatic clean up". Personally, I feel it's better for the threads to just be signaled to exit, so they can quit taking up valuable processing resources. But if you really insist on letting them just run, it's trivial to have an independent thread wait for them to finish and then dispose things properly at that point. For that matter, you can easily design a "last one out of the room, turn off the lights" implementation (see below). There is no need to dispose things right away, even if you want to keep the design simple. > Anyway you do it, it'll either put more effort into your design > (effort that isn't needed in the other design since this case doesn't > apply / or already implemented), or your pushing it onto the client of > the class to make sure the object can be disposed before they dispose. Neither is true. >> Your >> proposed design is inherently more complicated and requires more code to [quoted text clipped - 3 lines] > portion, vs whatever methods your creating (or automatic cleanup > logic). The automatic cleanup logic is minimal. No new methods are needed at all, and the logic can be as simple as decrementing a counter and cleaning up if it hits 0. For example, here's a version of your original example that delays disposal until all the threads are done, without imposing any new requirements on the client: class Class1 { private ReaderWriterLockSlim myStringLock = new ReaderWriterLockSlim(); private int _cthread; private volatile bool _fDispose; public void StartWork() { _cthread = 2; ThreadPool.QueueUserWorkItem(new WaitCallback(test)); ThreadPool.QueueUserWorkItem(new WaitCallback(test)); } public void test(Object nullState)< |
Enable EMail Alerts
Start New Thread
Reply to this Message