Michael Hall | 2 Mar 12:59

Re: issues in cvs

On Sun, 2003-03-02 at 11:27, Ed Sweetman wrote:
> Michael Hall wrote:
> > On Sat, 2003-03-01 at 17:50, Ed Sweetman wrote:
> > 
> >>I was able to recreate the bug and this latest cvs commit seems to fix 
> >>it. Try it out.
> >>
> >>somewhat unfortunately, my kernel caches all of filesystem data for my 
> >>music filesystem and subsequent attempts dont seem to touch the disc at 
> >>all and work completely from ram, even if it's a new zinf and i delete 
> >>the database it creates on just loading.
> >>
> > 
> > 
> > Just been looking over latest commit to musiccatalog.cpp (rev 1.11). It
> > seems that every method that once acquired the m_mutex now creates its
> > own Mutex (also called m_mutex, thus hiding the instance variable) and
> > acquires it. This means that every method call gets a different mutex...
> > kinda defeats the point, no?
> > 
> 
> you'd think so.  but I haven't seen any adverse effects from it and it 
> actually follows scope so it doesn't deadlock.

Given that everytime a method is called a new mutex is allocated off the
stack, I'm not surprised you don't get any deadlocks. You might as well
remove them completely. All they're doing now is slowing the code down
by acquiring a lock that it just created and no other code path is ever
going to see.

(Continue reading)

Ed Sweetman | 2 Mar 14:12
Favicon

Re: issues in cvs

Michael Hall wrote:
> On Sun, 2003-03-02 at 11:27, Ed Sweetman wrote:
> 
>>Michael Hall wrote:
>>
>>>On Sat, 2003-03-01 at 17:50, Ed Sweetman wrote:
>>>
>>>
>>>>I was able to recreate the bug and this latest cvs commit seems to fix 
>>>>it. Try it out.
>>>>
>>>>somewhat unfortunately, my kernel caches all of filesystem data for my 
>>>>music filesystem and subsequent attempts dont seem to touch the disc at 
>>>>all and work completely from ram, even if it's a new zinf and i delete 
>>>>the database it creates on just loading.
>>>>
>>>
>>>
>>>Just been looking over latest commit to musiccatalog.cpp (rev 1.11). It
>>>seems that every method that once acquired the m_mutex now creates its
>>>own Mutex (also called m_mutex, thus hiding the instance variable) and
>>>acquires it. This means that every method call gets a different mutex...
>>>kinda defeats the point, no?
>>>
>>
>>you'd think so.  but I haven't seen any adverse effects from it and it 
>>actually follows scope so it doesn't deadlock.
> 
> 
> Given that everytime a method is called a new mutex is allocated off the
(Continue reading)

Ed Sweetman | 2 Mar 23:07
Favicon

Re: issues in cvs

Sweet, pthread already does the job of creating a read write lock.

Our Mutex object can then goto this.

small and simple. this is it in it's entirety unless someone has any 
other suggestions.

class RWLock {
    public:
        RWLock(void) {
             pthread_rwlockattr_t attr;
             pthread_rwlockattr_init(&attr);
             pthread_rwlock_init(&lock, &attr);
             pthread_rwlockattr_destroy(&attr);
        }
        ~RWLock(void) { pthread_rwlock_destroy(&lock); }
        void RLock(void) { pthread_rwlock_rdlock(&lock);}
        void WLock(void) { pthread_rwlock_wrlock(&lock);}
        void Unlock(void) { pthread_rwlock_unlock(&lock); }
     private:
        pthread_rwlock_t lock;
};

if i impliment this, redo the locks that i changed to be readwrite locks 
and redo all the other locks in zinf to be readwrite locks and add more 
to where they should be (basically every damn place) we'll still have to 
deal with the semaphores. Hopefully this lock can make dealing with the 
semaphores no longer an issue.

i really do need help in this though.  zinf has like 150 source files 
(Continue reading)

Ed Sweetman | 3 Mar 04:05
Favicon

Re: issues in cvs

Ok, using the class below i've replaced our mutex class and am working 
my way through the /base directory tree.

there are a special cases we need to think about.

When we return a pointer from a function, the calling function must 
first call a lock function in the object that they're getting data from. 
then when it is safe to unlock that object we must call an unlock 
method.  this is the only way to protect the pointer.

Ed Sweetman wrote:
> Sweet, pthread already does the job of creating a read write lock.
> 
> Our Mutex object can then goto this.
> 
> small and simple. this is it in it's entirety unless someone has any 
> other suggestions.
> 
> class RWLock {
>    public:
>        RWLock(void) {
>             pthread_rwlockattr_t attr;
>             pthread_rwlockattr_init(&attr);
>             pthread_rwlock_init(&lock, &attr);
>             pthread_rwlockattr_destroy(&attr);
>        }
>        ~RWLock(void) { pthread_rwlock_destroy(&lock); }
>        void RLock(void) { pthread_rwlock_rdlock(&lock);}
>        void WLock(void) { pthread_rwlock_wrlock(&lock);}
>        void Unlock(void) { pthread_rwlock_unlock(&lock); }
(Continue reading)

Ed Sweetman | 4 Mar 08:26
Favicon

Re: issues in cvs

ok, i've been doing some work on creating a new locking mechanism that 
makes better sense for zinf.

I've merged the best of the rwlock with a sort of pseudorecursive locking.

To see how it works read the comments in the attached source files.

there is one achilles heal in the class, as you'll read in the files.
You cannot create a write lock on an object that the same thread has 
already created a read lock or you will deadlock.

other than that minor problem. There are really cool things about it
1. We do not use c++ STL primitives at all
2. We do not rely on semaphores and signals (this can lead to rare 
deadlocks which we are more than familiar with)
3. We are less than half the size of the previous mutex class when compiled
4. Less overhead to do to what we need to do.

I'm really confident that this class is superior to the old mutex class 
we used. Any comments against or just general comments about it are 
definitely needed.

Attachment (mutex.h): text/x-chdr, 1781 bytes
Attachment (mutex.cpp): text/x-c++src, 1506 bytes
Ed Sweetman | 6 Mar 02:02
Favicon

Re: issues in cvs

Since i got all that lovely feedback from everyone (/sarcasm) I'm moving 
further.

classes are going to be adapted to the new locking mechanism across the 
board.  Many will need added public methods in order to acquire either 
kind of lock and to unlock so that our methods that return pointers to 
private data members are secure.

Just a little something that needs to stay in mind when locking a class.

Always follow the code path of the method you are locking from a given 
thread.  in the course of that lock, if you are trying to readlock  you 
cannot have a write lock occur within it. This can occur if say, we call 
another public method that has a write lock in it.

An alternative to having locking instructions to itself in the methods 
is to only have public methods that lock and unlock and we rely on all 
calling methods that use another object to lock via these methods.  In 
this way we will rarely if ever have a situation where we have nested 
locks.

Ed Sweetman wrote:
> ok, i've been doing some work on creating a new locking mechanism that 
> makes better sense for zinf.
> 
> 
> I've merged the best of the rwlock with a sort of pseudorecursive locking.
> 
> To see how it works read the comments in the attached source files.
> 
(Continue reading)

Bruce Brown | 6 Mar 05:23

RE: issues in cvs


I've been mostly quiet, watching all this locking stuff transpire.
Now I feel compelled to say something :)

You''ve done a great job getting the locking to work; why do you want
to muddy it up by exposing the locking primitives as public methods?
You're going to end up with endless rounds of bugs involved with some
method forgetting to unlock things. Instead, create some buddy guard
classes that deal with unlocking in the _dtor and perhaps expose an
unlock method. Then pass an instance of the object to the guard
_ctor, and have the lock managed.

> -----Original Message-----
> From: zinf-devel-admin <at> lists.sourceforge.net
> [mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
> Sent: Wednesday, March 05, 2003 5:02 PM
> To: zinf-devel <at> lists.sourceforge.net
> Subject: Re: [Zinf-devel] issues in cvs
>
>
> Since i got all that lovely feedback from everyone (/sarcasm) I'm moving
> further.
>
> classes are going to be adapted to the new locking mechanism across the
> board.  Many will need added public methods in order to acquire either
> kind of lock and to unlock so that our methods that return pointers to
> private data members are secure.
>
> Just a little something that needs to stay in mind when locking a class.
>
(Continue reading)

Ed Sweetman | 6 Mar 05:56
Favicon

Re: issues in cvs

Bruce Brown wrote:
> I've been mostly quiet, watching all this locking stuff transpire.
> Now I feel compelled to say something :)
> 
> You''ve done a great job getting the locking to work; why do you want
> to muddy it up by exposing the locking primitives as public methods?
> You're going to end up with endless rounds of bugs involved with some
> method forgetting to unlock things. Instead, create some buddy guard
> classes that deal with unlocking in the _dtor and perhaps expose an
> unlock method. Then pass an instance of the object to the guard
> _ctor, and have the lock managed.
> 

it's a necessity that they be lockable by a class that is calling a 
function that returns a pointer to a private data member.

Not only that, but if i need to call a function that returns any value 
that is pertinant to the state or value of something in that object and 
i use that in my function and end up altering something outside of that 
function based on it, the object i queried in the beginning shouldn't be 
allowed to change, should it?

eg.
    thread1
	function asks for state of object in thread2
    thread3
	delete same object in thread2
    thread1
	if(state) call method in same object of thread2

(Continue reading)

Bruce Brown | 6 Mar 06:11

RE: issues in cvs


one of us missed something... I'll try to be brief.

In the method that requests data, I'd assume it does
the lock and unlock, otherwise you have a case of asymetric
locking where the the caller is either responsible for locking
or unlocking; it can be done, I've done it, but it isn't pretty.

So, prior to obtaining the data from the class object, the
method instantiates a guard object passing the class object
that will be locked. Then, the data is obtained either directly
(yuck) or through an accessor. At some point the guard object
goes out of scope and the lock is released. To be nice, the
guard should include an unlock. I wouldn't add a lock because
you get into recursive locking problems unless you ignore the
case of lock being called while a lock is held.

If you want to get fancier, you can pass an instance of the
guard to the accessor so that it can set object up to deal
with proper unlocking...this is what you need to do if some
way outer method wants to call deep down and guarantee that
a lock is held over the entire return path.

The point being that the guard is in the context of the
thread and object that is requesting data, not the object
giving the data up.

> -----Original Message-----
> From: zinf-devel-admin <at> lists.sourceforge.net
> [mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
(Continue reading)

Ed Sweetman | 6 Mar 06:57
Favicon

Re: issues in cvs

Bruce Brown wrote:
> one of us missed something... I'll try to be brief.
> 
> In the method that requests data, I'd assume it does
> the lock and unlock, otherwise you have a case of asymetric
> locking where the the caller is either responsible for locking
> or unlocking; it can be done, I've done it, but it isn't pretty
in the case of functions returning certain types of data where the 
caller would need the object locked while using the returned data, the 
caller  would be responsible for both locking and unlocking, we would 
not lock within the method but assume our caller has us locked.

> So, prior to obtaining the data from the class object, the
> method instantiates a guard object passing the class object
> that will be locked. Then, the data is obtained either directly
> (yuck) or through an accessor. At some point the guard object
> goes out of scope and the lock is released. To be nice, the
> guard should include an unlock. I wouldn't add a lock because
> you get into recursive locking problems unless you ignore the
> case of lock being called while a lock is held.

if you read my new lock mechanism comments, it is recursive-friendly.
The only recursive it's not friendly with is going from read to write 
locks. It can do read on read, write on write and read on write. It's a 
really sweet locking class.

> If you want to get fancier, you can pass an instance of the
> guard to the accessor so that it can set object up to deal
> with proper unlocking...this is what you need to do if some
> way outer method wants to call deep down and guarantee that
(Continue reading)

Bruce Brown | 6 Mar 07:16

RE: issues in cvs


> -----Original Message-----
> From: zinf-devel-admin <at> lists.sourceforge.net
> [mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
> Sent: Wednesday, March 05, 2003 9:58 PM
> To: zinf-devel <at> lists.sourceforge.net
> Subject: Re: [Zinf-devel] issues in cvs
>
>
> Bruce Brown wrote:
> > one of us missed something... I'll try to be brief.
> >
> > In the method that requests data, I'd assume it does
> > the lock and unlock, otherwise you have a case of asymetric
> > locking where the the caller is either responsible for locking
> > or unlocking; it can be done, I've done it, but it isn't pretty
> in the case of functions returning certain types of data where the
> caller would need the object locked while using the returned data, the
> caller  would be responsible for both locking and unlocking, we would
> not lock within the method but assume our caller has us locked.
>
> > So, prior to obtaining the data from the class object, the
> > method instantiates a guard object passing the class object
> > that will be locked. Then, the data is obtained either directly
> > (yuck) or through an accessor. At some point the guard object
> > goes out of scope and the lock is released. To be nice, the
> > guard should include an unlock. I wouldn't add a lock because
> > you get into recursive locking problems unless you ignore the
> > case of lock being called while a lock is held.
>
(Continue reading)

Ed Sweetman | 6 Mar 08:19
Favicon

Re: issues in cvs

Bruce Brown wrote:
> 
>>
>>Bruce Brown wrote:
>>
>>>one of us missed something... I'll try to be brief.
>>>
>>>In the method that requests data, I'd assume it does
>>>the lock and unlock, otherwise you have a case of asymetric
>>>locking where the the caller is either responsible for locking
>>>or unlocking; it can be done, I've done it, but it isn't pretty
>>
>>in the case of functions returning certain types of data where the
>>caller would need the object locked while using the returned data, the
>>caller  would be responsible for both locking and unlocking, we would
>>not lock within the method but assume our caller has us locked.
>>
>>
>>>So, prior to obtaining the data from the class object, the
>>>method instantiates a guard object passing the class object
>>>that will be locked. Then, the data is obtained either directly
>>>(yuck) or through an accessor. At some point the guard object
>>>goes out of scope and the lock is released. To be nice, the
>>>guard should include an unlock. I wouldn't add a lock because
>>>you get into recursive locking problems unless you ignore the
>>>case of lock being called while a lock is held.
>>
>>if you read my new lock mechanism comments, it is recursive-friendly.
>>The only recursive it's not friendly with is going from read to write
>>locks. It can do read on read, write on write and read on write. It's a
(Continue reading)

Bruce Brown | 6 Mar 09:43

RE: issues in cvs


I dind't mean that your locking was primitive. I meant
that it was the basis of a strong foundation, to be
layered upon: A Primitive.

I'm going on vaction in a few days, perhaps I'll feel
ambitious and provide an example of a set of lockable
classes with guards and a priority locking scheme...
maybe even a task class that keeps the priority straight.

> -----Original Message-----
> From: zinf-devel-admin <at> lists.sourceforge.net
> [mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
> Sent: Wednesday, March 05, 2003 11:20 PM
> To: zinf-devel <at> lists.sourceforge.net
> Subject: Re: [Zinf-devel] issues in cvs
>
>
> Bruce Brown wrote:
> >
> >>
> >>Bruce Brown wrote:
> >>
> >>>one of us missed something... I'll try to be brief.
> >>>
> >>>In the method that requests data, I'd assume it does
> >>>the lock and unlock, otherwise you have a case of asymetric
> >>>locking where the the caller is either responsible for locking
> >>>or unlocking; it can be done, I've done it, but it isn't pretty
> >>
(Continue reading)

Ed Sweetman | 6 Mar 12:06
Favicon

Re: issues in cvs

Bruce Brown wrote:
> I dind't mean that your locking was primitive. I meant
> that it was the basis of a strong foundation, to be
> layered upon: A Primitive.

pthread_rwlock is the primitive "the Primitive", I wasn't referring to 
you saying it was primitive as in basic, but as it was the building 
block. We dont need a layer on top to provide anything else.. maybe a 
function to return the type of lock held and who holds it for assert's 
purposes but a layer on top?  I dont think so.  rwlock already provides 
priority locking that's why we have two types of locks.  I'm not sure 
what you are aiming for with what you say is a priority scheme that 
isn't already provided by my class.

If you mean to return an error when a readlock is held by some thread on 
an object and we want to do a write lock,  that's perfectly valid, 
returning an error would be an error. Our write lock will wait until the 
read lock is unlocked.  Same thing if i write lock is held and we want 
to do a read lock.  Same thing if a write lock is held and we want to do 
a write lock.  But want a read lock on an object that is read locked 
will ignore eachother. This is all intended and good.  Recursive 
behavior was explained already, the only thing you cant do in a 
recursive setup is start out with a read lock and try doing a write lock.

What exactly would your priority scheme be providing that isn't already 
here? What exacly would a task class be providing that isn't already 
here.  the guard class scheme would provide scope abilities when we need 
it, that i understand the logic of and that actually provides a useful 
function that currently isn't possible with the way i was going to do 
things.
(Continue reading)

Bruce Brown | 6 Mar 09:22

RE: issues in cvs


> -----Original Message-----
> From: zinf-devel-admin <at> lists.sourceforge.net
> [mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
> Sent: Wednesday, March 05, 2003 11:20 PM
> To: zinf-devel <at> lists.sourceforge.net
> Subject: Re: [Zinf-devel] issues in cvs
>
>
> Bruce Brown wrote:
> >
> >>
> >>Bruce Brown wrote:
> >>
> >>>one of us missed something... I'll try to be brief.
> >>>
> >>>In the method that requests data, I'd assume it does
> >>>the lock and unlock, otherwise you have a case of asymetric
> >>>locking where the the caller is either responsible for locking
> >>>or unlocking; it can be done, I've done it, but it isn't pretty
> >>
> >>in the case of functions returning certain types of data where the
> >>caller would need the object locked while using the returned data, the
> >>caller  would be responsible for both locking and unlocking, we would
> >>not lock within the method but assume our caller has us locked.
> >>
> >>
> >>>So, prior to obtaining the data from the class object, the
> >>>method instantiates a guard object passing the class object
> >>>that will be locked. Then, the data is obtained either directly
(Continue reading)

Ed Sweetman | 6 Mar 11:48
Favicon

Re: issues in cvs

Bruce Brown wrote:
> 
>>-----Original Message-----
>>From: zinf-devel-admin <at> lists.sourceforge.net
>>[mailto:zinf-devel-admin <at> lists.sourceforge.net]On Behalf Of Ed Sweetman
>>Sent: Wednesday, March 05, 2003 11:20 PM
>>To: zinf-devel <at> lists.sourceforge.net
>>Subject: Re: [Zinf-devel] issues in cvs
>>
>>
>>Bruce Brown wrote:
>>
>>>>Bruce Brown wrote:
>>>>
>>>>
>>>>>one of us missed something... I'll try to be brief.
>>>>>
>>>>>In the method that requests data, I'd assume it does
>>>>>the lock and unlock, otherwise you have a case of asymetric
>>>>>locking where the the caller is either responsible for locking
>>>>>or unlocking; it can be done, I've done it, but it isn't pretty
>>>>
>>>>in the case of functions returning certain types of data where the
>>>>caller would need the object locked while using the returned data, the
>>>>caller  would be responsible for both locking and unlocking, we would
>>>>not lock within the method but assume our caller has us locked.
>>>>
>>>>
>>>>
>>>>>So, prior to obtaining the data from the class object, the
(Continue reading)


Gmane