[ 
https://issues.apache.org/jira/browse/QPID-3193?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13017239#comment-13017239
 ] 

Cliff Jansen edited comment on QPID-3193 at 4/8/11 2:28 AM:
------------------------------------------------------------

I think you are approaching this the wrong way.

As the owner of the managed/unmanaged interface, it is your responsibility to 
catch and trap exceptions that the user of your class can't handle.  Changing 
the semantics of Dispose() is not what the user would expect.

> A C# coder may innocently write:
[...]
> (N)  mB.Dispose();

The sample that you show works exactly as it should.  Replace your Message 
class with System.IO.Stream (or any other IDisposable class), and it will also 
fail when you try to access a feature that is undefined for the disposed object.

Conversely, if you faked the dispose because you somehow knew mA still wanted 
it, you would have robbed mB of it's attempt to optimize the use of the 
unmanaged resource.

If you absolutely need to to let the application know if the message is 
disposed, see

  
http://stackoverflow.com/questions/192206/how-does-one-tell-if-an-idisposable-object-reference-is-disposed

but it is rarely needed, otherwise it would be a commonly used pattern.  The 
user of mA would usually just use a try catch block if he is worried about an 
async dispose since:

{code}
  if (!mA.IsDisposed) {
    // another thread calls mB.Dispose() or mA.Dispose()
    mA.DoSomething();
  }
{code}

still leaves a window open.

> Another answer is to have the binding check for a null binding
> reference on each and every access and then to throw if the underlying
> binding is gone. This is not very appealing from a performance
> standpoint.

If your class is meant to be thread safe, you have to take a deeper performance 
hit and acquire a lock for the duration of test and use.  If you don't need 
thread safety, to avoid the test except on failure use:

{code}
  try {
    return messagep->foo();
  }
  catch (const std::exception& error) {
    if (someDisposedTestHere) {
      throw gcnew ObjectDisposedException("Message foo");
    else
      throw gcnew MyException(gcnew String(error.what()));
  }
{code}


I believe this should convert the null pointer (unmaged space) exception for 
the user into the expected managed space exception.

[sorry, forgot the code tags]


      was (Author: cliffjansen):
    I think you are approaching this the wrong way.

As the owner of the managed/unmanaged interface, it is your responsibility to 
catch and trap exceptions that the user of your class can't handle.  Changing 
the semantics of Dispose() is not what the user would expect.

> A C# coder may innocently write:
[...]
> (N)  mB.Dispose();

The sample that you show works exactly as it should.  Replace your Message 
class with System.IO.Stream (or any other IDisposable class), and it will also 
fail when you try to access a feature that is undefined for the disposed object.

Conversely, if you faked the dispose because you somehow knew mA still wanted 
it, you would have robbed mB of it's attempt to optimize the use of the 
unmanaged resource.

If you absolutely need to to let the application know if the message is 
disposed, see

  
http://stackoverflow.com/questions/192206/how-does-one-tell-if-an-idisposable-object-reference-is-disposed

but it is rarely needed, otherwise it would be a commonly used pattern.  The 
user of mA would usually just use a try catch block if he is worried about an 
async dispose since:

  if (!mA.IsDisposed) {
    // another thread calls mB.Dispose() or mA.Dispose()
    mA.DoSomething();
  }

still leaves a window open.

> Another answer is to have the binding check for a null binding
> reference on each and every access and then to throw if the underlying
> binding is gone. This is not very appealing from a performance
> standpoint.

If your class is meant to be thread safe, you have to take a deeper performance 
hit and acquire a lock for the duration of test and use.  If you don't need 
thread safety, to avoid the test except on failure use:

  try {
    return messagep->foo();
  }
  catch (const std::exception& error) {
    if (someDisposedTestHere) {
      throw gcnew ObjectDisposedException("Message foo");
    else
      throw gcnew MyException(gcnew String(error.what()));
  }


I believe this should convert the null pointer (unmaged space) exception for 
the user into the expected managed space exception.

  
> .NET Binding for Messaging classes need a test to check that binding is still 
> in effect
> ---------------------------------------------------------------------------------------
>
>                 Key: QPID-3193
>                 URL: https://issues.apache.org/jira/browse/QPID-3193
>             Project: Qpid
>          Issue Type: Improvement
>    Affects Versions: 0.11
>            Reporter: Chuck Rolke
>            Assignee: Chuck Rolke
>
> The .NET Binding for Messaging could be made more user-friendly with the 
> addition of a property that indicates whether or not the underlying binding 
> is still available. A C# coder may innocently write:
> (1)  Message mA = new Message("a");
> (2)  Message mB = mA;
>      ...
> (N)  mB.Dispose();
> After disposing of message mB then message mA is clobbered. 'Message' is a 
> 'ref class' type and messages mA and mB refer to the same object on managed 
> heap. When message mB is disposed then the bound C++ Messaging object is 
> deleted [1]. Any reference to the bound message part of mA will result in an 
> illegal memory reference (to 0) and a process exit. The .NET runtime can't 
> catch this fault.
> The obvious answer is not to do that. If the second line of code was 'Message 
> mB = new Message(mA)' then mA and mB would have been completely separate and 
> disposing of either would have no effect on the other.
> Another answer is to have the binding check for a null binding reference on 
> each and every access and then to throw if the underlying binding is gone. 
> This is not very appealing from a performance standpoint.
> As a compromise I would like to add a property isBound to each class. Users 
> then have a fighting chance to check that the binding is still in effect and 
> that function calls on the object shouldn't blow up. This property would be 
> useful in Assert statements or in debugging.
> [1] If anyone knows how to have my binding library intercept example code 
> line (2) and create reference counts, please let me know.

--
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:[email protected]

Reply via email to