is read/write mutex example correct

Selçuk Giray Özdamar

14 May 2008 14 May '08

1:42 p.m.

Hi everyone, Thanks for the prev. read/write mutex replies. I wrote a simple container which support concurrent read support and exclusive write support. Is this code correct, I havent write test code for it yet. Tnx... #pragma once #ifndef ENTITYUPDATEBUFFER_H___ #define ENTITYUPDATEBUFFER_H___ // std includes #include <vector> // boost includes #include template <typename T> class EntityUpdateBuffer { public: typedef typename boost::shared_lockboost::shared_mutex read_lock; typedef typename boost::unique_lockboost::shared_mutex write_lock; EntityUpdateBuffer(void){} ~EntityUpdateBuffer(void){} void push_back(const T& value); const T& at(int index) const; T& at(int index); private: std::vector<T> m_buffer; boost::shared_mutex rw_mutex; }; template <typename T> void EntityUpdateBuffer<T>::push_back( const T& value ) { // acquire lock write_lock lock(rw_mutex); m_buffer.push_back(value); } template <typename T> const T& EntityUpdateBuffer<T>::at( int index ) const { // acquire lock read_lock lock(rw_mutex); return m_buffer.at(index); } template <typename T> T& EntityUpdateBuffer<T>::at( int index ) { // acquire lock read_lock lock(rw_mutex); return m_buffer.at(index); } #endif //ENTITYUPDATEBUFFER_H___ ---Sgo---

Attachments:

attachment.html (text/html — 16.1 KB)

Show replies by date

Kevin Heifner

14 May 14 May

1:55 p.m.

Selçuk Giray Özdamar wrote:

...

const T& at(int index) const;

This doesn't look good.

...

T& at(int index);

Or this.

...

private: std::vector<T> m_buffer; boost::shared_mutex rw_mutex; };

...

template <typename T> const T& EntityUpdateBuffer<T>::at( int index ) const { // acquire lock read_lock lock(rw_mutex); return m_buffer.at(index); }

The problem with this code is that you are returning a reference to data that might be immediately invalidated. A push_back via another thread can cause the referenced memory to no longer be valid.

...

template <typename T> T& EntityUpdateBuffer<T>::at( int index ) { // acquire lock read_lock lock(rw_mutex); return m_buffer.at(index); }

Same problem. KevinH -- Kevin Heifner heifner @ ociweb.com http://heifner.blogspot.com Object Computing, Inc. (OCI) www.ociweb.com

Igor R.

2:03 p.m.

...

...
The problem with this code is that you are returning a reference > to data that might be immediately invalidated. A push_back via > another thread can cause the referenced memory to no longer be valid. Does push_back() really invalidate iterators to *previous* vector elements?!

News, entertainment and everything you care about at Live.com. Get it now! http://www.live.com/getstarted.aspx

Anthony Williams

2:14 p.m.

Igor R. writes:

...

...
...
The problem with this code is that you are returning a reference > to data that might be immediately invalidated. A push_back via > another thread can cause the referenced memory to no longer be valid. Does push_back() really invalidate iterators to *previous* vector elements?!

Yes: it might have to allocate a new chunk of memory for the vector, in which case it will have to move all the existing elements to the new memory, and free the old one. Anthony -- Anthony Williams | Just Software Solutions Ltd Custom Software Development | http://www.justsoftwaresolutions.co.uk Registered in England, Company Number 5478976. Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

dhruva

15 May 15 May

11:23 a.m.

Hi, On Wed, May 14, 2008 at 7:44 PM, Anthony Williams wrote:

...

Igor R. writes:

...
...
...
The problem with this code is that you are returning a reference > to data that might be immediately invalidated. A push_back via > another thread can cause the referenced memory to no longer be valid. Does push_back() really invalidate iterators to *previous* vector elements?!

Yes: it might have to allocate a new chunk of memory for the vector, in which case it will have to move all the existing elements to the new memory, and free the old one.

The write locks and read locks in your code are identical (shared mutex). On a call that does a push into the vector, you are modifying data or doing a write. You should upgrade the lock you are holding from a read to a write lock by a call to lock_upgrade (gets the thread ability to gain exclusive lock) followed by unlock_upgrade_and_lock() (gives up the ability and gains the exclusive lock atomically) (I may be wrong here, requesting others to chip in), do the push and call to unlock_upgrade(). That will be a true implementation of single write and multiple reader paradigm. -dhruva -- Contents reflect my personal views only!

Igor R.

16 May 16 May

11:04 a.m.

I see... Then the whole problem could be solved just by using deque instead of vector - since deque does't guarantee contiguous memory block for its elements, and thus it shouldn't reallocate the storage.> >> > The problem with this code is that you are returning a reference > to data that might be immediately invalidated. A push_back via > another thread can cause the referenced memory to no longer be valid.> > Does push_back() really invalidate iterators to *previous* vector elements?!> > Yes: it might have to allocate a new chunk of memory for the vector, in which> case it will have to move all the existing elements to the new memory, and> free the old one.> > Anthony _________________________________________________________________ News, entertainment and everything you care about at Live.com. Get it now! http://www.live.com/getstarted.aspx

Patrick Loney

11:07 a.m.

What you need is something that guarantees not to move its elements rather than something that doesn't guarantee contiguous memory. I don't know if deque does this but you could end up with code that only works if it's built with the right compiler... ________________________________ From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Igor R. Sent: 16 May 2008 12:04 To: boost-users@lists.boost.org Subject: Re: [Boost-users] is read/write mutex example correct I see... Then the whole problem could be solved just by using deque instead of vector - since deque does't guarantee contiguous memory block for its elements, and thus it shouldn't reallocate the storage.

...

...
...
...
The problem with this code is that you are returning a reference to data that might be immediately invalidated. A push_back via > another thread can cause the referenced memory to no longer be valid. Does push_back() really invalidate iterators to *previous* vector elements?!

Yes: it might have to allocate a new chunk of memory for the vector, in which case it will have to move all the existing elements to the new memory, and free the old one.

Anthony

________________________________ Get news, entertainment and everything you care about at Live.com. Check it out! http://www.live.com/getstarted.aspx%20 ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________ ****************************************************************************** "This message and any attachments are solely for the intended recipient and may contain confidential and privileged information. If you are not the intended recipient, any disclosure, copying, use, or distribution of the information included in this message and any attachments is prohibited. If you have received this communication in error, please notify us by reply e-mail and immediately and permanently delete this message and any attachments. Thank you." Interactive Transaction Solutions Ltd (2473364 England) Registered Office: Systems House, Station Approach Emsworth PO10 7PW ********************************************************************** Ce message �lectronique contient des informations confidentielles � l'usage unique des destinataires indiqu�s, personnes physiques ou morales. Si vous n'�tes pas le destinataire voulu, toute divulgation, copie, ou diffusion ou toute autre utilisation de ces informations, est interdite. Si vous avez re�u ce message �lectronique par erreur, nous vous remercions d'en avertir son exp�diteur imm�diatement par email et de d�truire ce message ainsi que les �l�ments attach�s. Interactive transaction Solutions SAS- France (RCS Pontoise : 489 397 877) Si�ge social : Parc Saint Christophe, 10, Avenue de l�Entreprise 95865 Cergy-Pontoise Cedex ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________

Kevin Heifner

11:23 a.m.

Patrick Loney wrote:

...

What you need is something that guarantees not to move its elements rather than something that doesn’t guarantee contiguous memory. I don’t know if deque does this but you could end up with code that only works if it’s built with the right compiler…

The spec says for a deque, "An insert at either end of the deque invalidates all the iterators to the deque, but has no effect on the validity of references to elements of the deque." However, is the requirement to only append and never erase? That seems unlikely. KevinH -- Kevin Heifner heifner @ ociweb.com http://heifner.blogspot.com Object Computing, Inc. (OCI) www.ociweb.com

Frank Mori Hess

14 May 14 May

2:16 p.m.

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Wednesday 14 May 2008 10:03 am, Igor R. wrote:

...

...
The problem with this code is that you are returning a reference to data that might be immediately invalidated. A push_back via another thread can cause the referenced memory to no longer be valid.

Does push_back() really invalidate iterators to *previous* vector elements?!

It can, if it causes the vector to reallocate memory. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFIKvQv5vihyNWuA4URAiFnAKDVpSdlLmqGSCuxOPH4WitCpgiPNQCcCfC3 TkZFyiDH5n10xeZrF6soptc= =hxAV -----END PGP SIGNATURE-----

Kevin Heifner

2:24 p.m.

Igor R. wrote:

...

...
The problem with this code is that you are returning a reference to data that might be immediately invalidated. A push_back via another thread can cause the referenced memory to no longer be valid.

Does push_back() really invalidate iterators to *previous* vector elements?!

Yes, and I quote the spec for insert which push_back() is based on: "Remarks: Causes reallocation if the new size is greater than the old capacity. If no reallocation happens, all the iterators and references before the insertion point remain valid." So unless you set capacity of the vector so that it never grows then push_back() can invalidate a held reference. Also it probably should be noted that even if you setup capacity so that the vector never grows and only allowed push_back() you still need mutex guards around any access to the shared object. KevinH -- Kevin Heifner heifner @ ociweb.com http://heifner.blogspot.com Object Computing, Inc. (OCI) www.ociweb.com

6127

Age (days ago)

6129

Last active (days ago)

List overview

Download

9 comments

7 participants

participants (7)

Anthony Williams
dhruva
Frank Mori Hess
Igor R.
Kevin Heifner
Patrick Loney
Selçuk Giray Özdamar