TapiLine.Close() can raise ObjectDisposedException. Is there a race in SafeHandle?

Nov 23, 2012 at 2:00 PM

Hi guys,
first of all, great work! Library works fine and seems pretty stable to us. 

By the way, we're randomly facing an issue that smells very much like a bug/race. 

Below, there are 2 log lines produced by our application, that receives a LINE REMOVE event for a given TAPI line. Our app reacts invoking Close() on the line just removed. 99% of times it works fine. Randomly, we receive an ObjectDisposedException, saying "Safe handle has been closed". 

Actually, this is not causing a real issue to our app, but we investigated ATAPI source and discovered something we don't understand and we want to share with you.

11-16 17:15:33.276 - DEBUG - [159]: (MyTapiLine) - LINE_REMOVE tapi event received for line ... 
11-16 17:15:33.276 - ERROR - [159]: (MyTapiLine) - Generic exception closing tapi line: System.ObjectDisposedException: Safe handle has been closed 
at System.Runtime.InteropServices.SafeHandle.DangerousAddRef(Boolean& success) 
at System.StubHelpers.StubHelpers.SafeHandleAddRef(SafeHandle pHandle, Boolean& success) 
at JulMar.Atapi.Interop.NativeMethods.lineGetLineDevStatus(HTLINE htLine, IntPtr lpLineStatus) 
at JulMar.Atapi.TapiLine.GatherStatus() in TapiLine.cs:line 895 
at JulMar.Atapi.TapiLine.Close() in TapiLine.cs:line 1236 
at MyTapiLine.Close() 

Our analysys: the exception was raised because a native TAPI method, lineGetLineDevStatus(), has been invoked within TapiLine.GatherStatus() method, that passed an invalid handle. And GatherStatus() was called within TapiLine.Close() method, after closing the handle and marking it as invalid. But going back to GatherStatus(), we think that, in the original intent, it should had exited at the initial the IsOpen() check, avoiding the invocation of the TAPI method.

Summurizing, when TapiLine.Close() is invoked by external app:

  1. line's handle is closed and marked as invalid
  2. but, suddenly the same handle seems still valid (IsOpen() within GatherStatus())
  3. so, a TAPI method is invoked with an handle, that is actually invalid. 
  4. Thus, the exception

As this is not involving the underlying TAPI provider, nor our app, and moreover it does happen randomly, we're thinking at some kind of race within the SafeHandle class.

 

Thanks in advance and regards.

Marco.

Nov 27, 2012 at 2:55 PM
Hi Marco,

Your analysis sounds right on to me - I've not looked at the source in a long time, but it's very possible there is an issue there that we never really encountered; if you decide to fix it, please submit a patch and I'll push it into the source!

Thanks,

Mark Smith

mark@julmar.com | @marksm | www.julmar.com/blog/mark




On Fri, Nov 23, 2012 at 6:01 AM, rum <notifications@codeplex.com> wrote:

From: rum

Hi guys,
first of all, great work! Library works fine and seems pretty stable to us.

By the way, we're randomly facing an issue that smells very much like a bug/race.

Below, there are 2 log lines produced by our application, that receives a LINE REMOVE event for a given TAPI line. Our app reacts invoking Close() on the line just removed. 99% of times it works fine. Randomly, we receive an ObjectDisposedException, saying "Safe handle has been closed".

Actually, this is not causing a real issue to our app, but we investigated ATAPI source and discovered something we don't understand and we want to share with you.

11-16 17:15:33.276 - DEBUG - [159]: (MyTapiLine) - LINE_REMOVE tapi event received for line ... 
11-16 17:15:33.276 - ERROR - [159]: (MyTapiLine) - Generic exception closing tapi line: System.ObjectDisposedException: Safe handle has been closed 
at System.Runtime.InteropServices.SafeHandle.DangerousAddRef(Boolean& success) 
at System.StubHelpers.StubHelpers.SafeHandleAddRef(SafeHandle pHandle, Boolean& success) 
at JulMar.Atapi.Interop.NativeMethods.lineGetLineDevStatus(HTLINE htLine, IntPtr lpLineStatus) 
at JulMar.Atapi.TapiLine.GatherStatus() in TapiLine.cs:line 895 
at JulMar.Atapi.TapiLine.Close() in TapiLine.cs:line 1236 
at MyTapiLine.Close() 

Our analysys: the exception was raised because a native TAPI method, lineGetLineDevStatus(), has been invoked within TapiLine.GatherStatus() method, that passed an invalid handle. And GatherStatus() was called within TapiLine.Close() method, after closing the handle and marking it as invalid. But going back to GatherStatus(), we think that, in the original intent, it should had exited at the initial the IsOpen() check, avoiding the invocation of the TAPI method.

Summurizing, when TapiLine.Close() is invoked by external app:

  1. line's handle is closed and marked as invalid
  2. but, suddenly the same handle seems still valid (IsOpen() within GatherStatus())
  3. so, a TAPI method is invoked with an handle, that is actually invalid.
  4. Thus, the exception

As this is not involving the underlying TAPI provider, nor our app, and moreover it does happen randomly, we're thinking at some kind of race within the SafeHandle class.

Thanks in advance and regards.

Marco.

Read the full discussion online.

To add a post to this discussion, reply to this email (atapi@discussions.codeplex.com)

To start a new discussion for this project, email atapi@discussions.codeplex.com

You are receiving this email because you subscribed to this discussion on CodePlex. You can unsubscribe or change your settings on codePlex.com.

Please note: Images and attachments will be removed from emails. Any posts to this discussion will also be available online at codeplex.com


Nov 29, 2012 at 8:28 AM
Edited Nov 29, 2012 at 9:06 AM

Hi Mark, 

Yes, I'm trying to fix it...

My focus, at the moment, is on the behavior of the HTLINE class. Actually, I don't have any personal experience about SafeHandle class, but, of course, I am quickly studying... For example, I was wondering why sometime the SetHandleAsInvalid() methos is called, and  sometime else, the Close() method is called.

Take the TapiCall class, ForceClose() method invokes SetHandleAsInvalid() only, while Deallocate() invokes Close() on the handle. On the other hand, the Close() method of TapiLine class invokes both Close() and SetHandleAsInvalid(). 

Looking at the source code for SafeHandle.cs class, a comment on the method SetHandleAsInvalid() states "This method will normally leak handles!".

Any hint about that?

Thx, Marco.

Jan 2, 2013 at 9:40 AM
Edited Jan 2, 2013 at 9:41 AM

Created issue 11471 and checked in a patch 82106.