You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Andrew Stitcher (JIRA)" <ji...@apache.org> on 2011/04/11 22:58:05 UTC

[jira] [Created] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
----------------------------------------------------------------------

                 Key: QPID-3199
                 URL: https://issues.apache.org/jira/browse/QPID-3199
             Project: Qpid
          Issue Type: Bug
          Components: C++ Broker, C++ Client
    Affects Versions: 0.8, 0.9
            Reporter: Andrew Stitcher
            Assignee: Andrew Stitcher
             Fix For: 0.10


The qpid::sys::StateMonitor class has 4 member functions liker this:
  void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }

However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.

What was meant was:
  void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }

which creates a local variable w which is destroyed at the end of the function execution unlocking this.

It is possible that the compiler/options we use do not actually exhibit this bug or at least only with some options as this correct behaviour, may not actually be implemented by all compilers.

If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.

This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Updated] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew Stitcher updated QPID-3199:
----------------------------------

    Fix Version/s: 0.10

> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.10, 0.11
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Commented] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Gordon Sim (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13018784#comment-13018784 ] 

Gordon Sim commented on QPID-3199:
----------------------------------

fwiw: I see no risk in including this in 0.10 providing there is still time to do so.

> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.11
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Commented] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Justin Ross (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13019038#comment-13019038 ] 

Justin Ross commented on QPID-3199:
-----------------------------------

Approved for 0.10.  Reviewed by Gordon Sim and Alan Conway.

> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.11
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Updated] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew Stitcher updated QPID-3199:
----------------------------------

    Description: 
The qpid::sys::StateMonitor class has 4 member functions liker this:
  void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }

However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.

What was meant was:
  void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }

which creates a local variable w which is destroyed at the end of the function execution unlocking this.

It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.

If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.

This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

  was:
The qpid::sys::StateMonitor class has 4 member functions liker this:
  void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }

However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.

What was meant was:
  void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }

which creates a local variable w which is destroyed at the end of the function execution unlocking this.

It is possible that the compiler/options we use do not actually exhibit this bug or at least only with some options as this correct behaviour, may not actually be implemented by all compilers.

If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.

This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.


> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>             Fix For: 0.10
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Commented] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Alan Conway (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13019034#comment-13019034 ] 

Alan Conway commented on QPID-3199:
-----------------------------------

I agree with Gordon. There's no risk and the bug potentially could cause very difficult to debug problems.

> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.11
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Resolved] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew Stitcher resolved QPID-3199.
-----------------------------------

       Resolution: Fixed
    Fix Version/s:     (was: 0.10)
                   0.11

Fixed on trunk

> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.11
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


[jira] [Commented] (QPID-3199) Severe but difficult to diagnose lock error in qpid::sys::StateMonitor

Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-3199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13018581#comment-13018581 ] 

Andrew Stitcher commented on QPID-3199:
---------------------------------------

Using this test program I established that gcc 4.5 does in fact perform as per the spec and so this is an actual bug in our code (maybe not triggered).

#include <iostream>
#include <string>

using std::cout;
using std::string;

class Scoped {
  string s;

public:
  Scoped(const string& s0) : 
    s(s0)
  {
    cout << "Scoped const" << s << "\n";
  }

  ~Scoped () {
    cout << "Scoped destr" << s << "\n";
  }
};

int main() {
  Scoped("A"); cout << "after A\n";
  Scoped s("B"); cout << "after B\n";
}

with results:

$ make CXXFLAGS=-O3 destructor-sequence
g++ -O3    destructor-sequence.cpp   -o destructor-sequence
$ ./destructor-sequence 
Scoped constA
Scoped destrA
after A
Scoped constB
after B
Scoped destrB
$ 


> Severe but difficult to diagnose lock error in qpid::sys::StateMonitor
> ----------------------------------------------------------------------
>
>                 Key: QPID-3199
>                 URL: https://issues.apache.org/jira/browse/QPID-3199
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker, C++ Client
>    Affects Versions: 0.8, 0.9
>            Reporter: Andrew Stitcher
>            Assignee: Andrew Stitcher
>              Labels: clang
>             Fix For: 0.10
>
>
> The qpid::sys::StateMonitor class has 4 member functions liker this:
>   void waitFor(Enum s) { ScopedWait(*this); while (s != state) wait(); }
> However the ScopeWait(*this); is in error, because it creates a temporary ScopedWait and then immediately destroys it. Instead of locking this and then unlocking it at the end of the function execution.
> What was meant was:
>   void waitFor(Enum s) { ScopedWait w(*this); while (s != state) wait(); }
> which creates a local variable w which is destroyed at the end of the function execution unlocking this.
> It is possible that the compiler/options we use do not actually exhibit this bug. As the correct behaviour may not actually be implemented by all compilers.
> If this bug exhibits it might show as StateMonitor::waitFor() never detecting the desired state for instance. It is remarkable that this code has been in the codebase for 3.5 years and we've not noticed any bug caused by it.
> This bug was detected by the work on QPID-3004: The clang++ compiler detected that the value being returned by the constructor was not being used at all.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org