Re: [Mono-dev] OracleClient.Oci and GC
Yeah, it is a sealed class which is why I didn't do protected virtual, so I'm wondering to what your initial remark referred: "Idiomatic IDisposable implementation is slightly different from what you have." On Aug 26, 2014, at 1:09 AM, Jonathan Pryor wrote: > On Aug 25, 2014, at 2:05 PM, Neale Ferguson wrote: >> Do you mean mine not having protected virtual? > > Dispose(bool) doesn't need to be `protected virtual` unless you plan on > supporting inheritance. > > Rephrased: if your type is sealed, it doesn't need to be `protected virtual`. > If your type *isn't* sealed, your type probably should provide a `protected > virtual void Dispose(bool disposing)`. > > - Jon > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
On Aug 25, 2014, at 2:05 PM, Neale Ferguson wrote: > Do you mean mine not having protected virtual? Dispose(bool) doesn't need to be `protected virtual` unless you plan on supporting inheritance. Rephrased: if your type is sealed, it doesn't need to be `protected virtual`. If your type *isn't* sealed, your type probably should provide a `protected virtual void Dispose(bool disposing)`. - Jon ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Idiomatic IDisposable implementation is slightly different from what you have: http://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx On Aug 25, 2014, at 11:08 AM, Neale Ferguson wrote: > I implemented a Dispose method for OracleParameter: > >~OracleParameter () >{ >Dispose(false); >} > >public void Dispose () >{ >Dispose (true); You should call GC.SuppressFinalize(this) from Dispose(), not Dispose(bool). - Jon ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Do you mean mine not having protected virtual? On Aug 25, 2014, at 2:00 PM, Jonathan Pryor wrote: > Idiomatic IDisposable implementation is slightly different from what you have: > > > http://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Finalization is not deterministic, it depends on the GC been able to collect all related objects. Maybe you have things keeping some of those 700 objects around? The way I test those things in a way that is reasonably reliable is: var t = new Thread (myTest); t.Start (); t.Join (); GC.Collect (); GC.WaitForPendingFinalizers (); //now verify the result. On Mon, Aug 25, 2014 at 11:08 AM, Neale Ferguson wrote: > I implemented a Dispose method for OracleParameter: > > ~OracleParameter () > { > Dispose(false); > } > > public void Dispose () > { > Dispose (true); > } > > void Dispose (bool disposing) > { > if (disposing) { > GC.SuppressFinalize(this); > } > if (indicator != IntPtr.Zero) { > Marshal.FreeHGlobal (indicator); > indicator = IntPtr.Zero; > } > } > > However, when I run the test program I see 1251 constructors being run but > only 572 destructors/disposals. How do I reconcile this disparity? > > Neale > > On Aug 22, 2014, at 11:40 AM, Neale Ferguson > wrote: > > > Thanks. I've made changes to OciDefineHandle and have cured nearly all > the failures I had been experiencing. I have started on OracleParameter as > it has an object called indicator that is used by the OCIBindxxx calls and > appears to suffer from the same problem. I was allocating the object in > native memory in the constructors and was going to free them in a > destructor. However, in my test runs I see I'm allocating 1200+ objects but > only freeing ~900. Would you elaborate on your final comment "failing to > dispose..." as I'm reading this as I don't need to Marshal.FreeHGlobal() > the object I allocated in the constructors. > > > > Neale > > > > On Aug 22, 2014, at 11:31 AM, Rodrigo Kumpera wrote: > > > >> Hey Neale, > >> > >> You can safely pass interior pointers to pinvoke without fearing the > object been collected for the duration of the call. > >> Mind that you have to correctly use specify in/out/ref depending on the > copy semantics you need. > >> > >> Problems only arise when native keeps that pointer after the call > finishes, this can result in the object been moved > >> as the GC has no visibility into the native heap. > >> > >> For those cases you can either create a pinning GC handle to the victim > object or you can allocate a chunk of native > >> memory that will be shared between managed and native to store the > desired value. Both options suck, TBH. > >> > >> I'd say go with the native chunk of code if you can't lexically scope > the pinning regions, it will be more reliable as > >> failing to dispose the object won't lead to permanent leaks. > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Are you explicitly calling dispose? Finalizers May not have been run yet depending on gc etc On Monday, August 25, 2014, Neale Ferguson wrote: > I implemented a Dispose method for OracleParameter: > > ~OracleParameter () > { > Dispose(false); > } > > public void Dispose () > { > Dispose (true); > } > > void Dispose (bool disposing) > { > if (disposing) { > GC.SuppressFinalize(this); > } > if (indicator != IntPtr.Zero) { > Marshal.FreeHGlobal (indicator); > indicator = IntPtr.Zero; > } > } > > However, when I run the test program I see 1251 constructors being run but > only 572 destructors/disposals. How do I reconcile this disparity? > > Neale > > On Aug 22, 2014, at 11:40 AM, Neale Ferguson > wrote: > > > Thanks. I've made changes to OciDefineHandle and have cured nearly all > the failures I had been experiencing. I have started on OracleParameter as > it has an object called indicator that is used by the OCIBindxxx calls and > appears to suffer from the same problem. I was allocating the object in > native memory in the constructors and was going to free them in a > destructor. However, in my test runs I see I'm allocating 1200+ objects but > only freeing ~900. Would you elaborate on your final comment "failing to > dispose..." as I'm reading this as I don't need to Marshal.FreeHGlobal() > the object I allocated in the constructors. > > > > Neale > > > > On Aug 22, 2014, at 11:31 AM, Rodrigo Kumpera > wrote: > > > >> Hey Neale, > >> > >> You can safely pass interior pointers to pinvoke without fearing the > object been collected for the duration of the call. > >> Mind that you have to correctly use specify in/out/ref depending on the > copy semantics you need. > >> > >> Problems only arise when native keeps that pointer after the call > finishes, this can result in the object been moved > >> as the GC has no visibility into the native heap. > >> > >> For those cases you can either create a pinning GC handle to the victim > object or you can allocate a chunk of native > >> memory that will be shared between managed and native to store the > desired value. Both options suck, TBH. > >> > >> I'd say go with the native chunk of code if you can't lexically scope > the pinning regions, it will be more reliable as > >> failing to dispose the object won't lead to permanent leaks. > > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > -- Studying for the Turing test ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
I implemented a Dispose method for OracleParameter: ~OracleParameter () { Dispose(false); } public void Dispose () { Dispose (true); } void Dispose (bool disposing) { if (disposing) { GC.SuppressFinalize(this); } if (indicator != IntPtr.Zero) { Marshal.FreeHGlobal (indicator); indicator = IntPtr.Zero; } } However, when I run the test program I see 1251 constructors being run but only 572 destructors/disposals. How do I reconcile this disparity? Neale On Aug 22, 2014, at 11:40 AM, Neale Ferguson wrote: > Thanks. I've made changes to OciDefineHandle and have cured nearly all the > failures I had been experiencing. I have started on OracleParameter as it has > an object called indicator that is used by the OCIBindxxx calls and appears > to suffer from the same problem. I was allocating the object in native memory > in the constructors and was going to free them in a destructor. However, in > my test runs I see I'm allocating 1200+ objects but only freeing ~900. Would > you elaborate on your final comment "failing to dispose..." as I'm reading > this as I don't need to Marshal.FreeHGlobal() the object I allocated in the > constructors. > > Neale > > On Aug 22, 2014, at 11:31 AM, Rodrigo Kumpera wrote: > >> Hey Neale, >> >> You can safely pass interior pointers to pinvoke without fearing the object >> been collected for the duration of the call. >> Mind that you have to correctly use specify in/out/ref depending on the copy >> semantics you need. >> >> Problems only arise when native keeps that pointer after the call finishes, >> this can result in the object been moved >> as the GC has no visibility into the native heap. >> >> For those cases you can either create a pinning GC handle to the victim >> object or you can allocate a chunk of native >> memory that will be shared between managed and native to store the desired >> value. Both options suck, TBH. >> >> I'd say go with the native chunk of code if you can't lexically scope the >> pinning regions, it will be more reliable as >> failing to dispose the object won't lead to permanent leaks. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Thanks. I've made changes to OciDefineHandle and have cured nearly all the failures I had been experiencing. I have started on OracleParameter as it has an object called indicator that is used by the OCIBindxxx calls and appears to suffer from the same problem. I was allocating the object in native memory in the constructors and was going to free them in a destructor. However, in my test runs I see I'm allocating 1200+ objects but only freeing ~900. Would you elaborate on your final comment "failing to dispose..." as I'm reading this as I don't need to Marshal.FreeHGlobal() the object I allocated in the constructors. Neale On Aug 22, 2014, at 11:31 AM, Rodrigo Kumpera wrote: > Hey Neale, > > You can safely pass interior pointers to pinvoke without fearing the object > been collected for the duration of the call. > Mind that you have to correctly use specify in/out/ref depending on the copy > semantics you need. > > Problems only arise when native keeps that pointer after the call finishes, > this can result in the object been moved > as the GC has no visibility into the native heap. > > For those cases you can either create a pinning GC handle to the victim > object or you can allocate a chunk of native > memory that will be shared between managed and native to store the desired > value. Both options suck, TBH. > > I'd say go with the native chunk of code if you can't lexically scope the > pinning regions, it will be more reliable as > failing to dispose the object won't lead to permanent leaks. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Hey Neale, You can safely pass interior pointers to pinvoke without fearing the object been collected for the duration of the call. Mind that you have to correctly use specify in/out/ref depending on the copy semantics you need. Problems only arise when native keeps that pointer after the call finishes, this can result in the object been moved as the GC has no visibility into the native heap. For those cases you can either create a pinning GC handle to the victim object or you can allocate a chunk of native memory that will be shared between managed and native to store the desired value. Both options suck, TBH. I'd say go with the native chunk of code if you can't lexically scope the pinning regions, it will be more reliable as failing to dispose the object won't lead to permanent leaks. On Fri, Aug 22, 2014 at 11:11 AM, Neale Ferguson wrote: > Is that just a comment on my ref IntPtr question or the use of ref with > the OCI stuff in general? > > On Aug 22, 2014, at 10:45 AM, Rodrigo Kumpera wrote: > > > Mono does conservative scanning of the native stack, so once the pointer > has crossed over to native, the containing object will remain pinned. > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Is that just a comment on my ref IntPtr question or the use of ref with the OCI stuff in general? On Aug 22, 2014, at 10:45 AM, Rodrigo Kumpera wrote: > Mono does conservative scanning of the native stack, so once the pointer has > crossed over to native, the containing object will remain pinned. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Mono does conservative scanning of the native stack, so once the pointer has crossed over to native, the containing object will remain pinned. On Fri, Aug 22, 2014 at 9:45 AM, Neale Ferguson wrote: > I am also wondering about the parameters in some of the OCIBind > methods and the OCIDefineByPosPtr that are "ref IntPtr". There's a small > window when GC could suspend the thread where the IntPtr object is also > moved before the OCI call has completed and set the underlying IntPtr field. > > On Aug 21, 2014, at 9:48 PM, Miguel de Icaza wrote: > > Hello, > > Wanted to follow up to Neale's comment, as he clarified an important point > that I overlooked. > > There are two ref parameters that are being passed to unmanaged code, both > point to fields inside the OciDefineHandle managed type. > > Neale's analysis is correct: the object might move and with it, the two > short variables that were passed to OciDefineByPos. This would explain > the crashes that developers are experiencing with the OracleClient provider > and Sgen. > > The proposed fix is one possible solution: to allocate the values outside > of the managed heap for both the short values. > > There is another suspicious use that we should look into. The > OciDefineByPos method is actually a set of overloaded methods, with > different types for the "value" parameter.It is often a handle that is > usually allocated via an unmanaged API. But there are a few troubling > uses: > >- ref value: used in DefineTimeStamp >- ref value: when passing the reference to a Handle defined in a >separate class, DefineLob, DefineInterval > > The above does not look very easy to fix. > > Given that these are resources that should be explicitly disposed, perhaps > what we should do is allocate a GCHandle for the OciDefineHandle object > (from OciStatementHandle, the one place that creates these objects) and > also create GCHandles for the objects that we use their Handle fields from > (in DefineTimeStamp, DefineLob, DefineInterval) and release the GCHandles > in the respective Dispose methods. > > Miguel > > > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
yeah, that makes sense. On Fri, Aug 22, 2014 at 9:45 AM, Neale Ferguson wrote: > I am also wondering about the parameters in some of the OCIBind > methods and the OCIDefineByPosPtr that are "ref IntPtr". There's a small > window when GC could suspend the thread where the IntPtr object is also > moved before the OCI call has completed and set the underlying IntPtr field. > > On Aug 21, 2014, at 9:48 PM, Miguel de Icaza wrote: > > Hello, > > Wanted to follow up to Neale's comment, as he clarified an important point > that I overlooked. > > There are two ref parameters that are being passed to unmanaged code, both > point to fields inside the OciDefineHandle managed type. > > Neale's analysis is correct: the object might move and with it, the two > short variables that were passed to OciDefineByPos. This would explain > the crashes that developers are experiencing with the OracleClient provider > and Sgen. > > The proposed fix is one possible solution: to allocate the values outside > of the managed heap for both the short values. > > There is another suspicious use that we should look into. The > OciDefineByPos method is actually a set of overloaded methods, with > different types for the "value" parameter.It is often a handle that is > usually allocated via an unmanaged API. But there are a few troubling > uses: > >- ref value: used in DefineTimeStamp >- ref value: when passing the reference to a Handle defined in a >separate class, DefineLob, DefineInterval > > The above does not look very easy to fix. > > Given that these are resources that should be explicitly disposed, perhaps > what we should do is allocate a GCHandle for the OciDefineHandle object > (from OciStatementHandle, the one place that creates these objects) and > also create GCHandles for the objects that we use their Handle fields from > (in DefineTimeStamp, DefineLob, DefineInterval) and release the GCHandles > in the respective Dispose methods. > > Miguel > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
I am also wondering about the parameters in some of the OCIBind methods and the OCIDefineByPosPtr that are "ref IntPtr". There's a small window when GC could suspend the thread where the IntPtr object is also moved before the OCI call has completed and set the underlying IntPtr field. On Aug 21, 2014, at 9:48 PM, Miguel de Icaza wrote: > Hello, > > Wanted to follow up to Neale's comment, as he clarified an important point > that I overlooked. > > There are two ref parameters that are being passed to unmanaged code, both > point to fields inside the OciDefineHandle managed type. > > Neale's analysis is correct: the object might move and with it, the two short > variables that were passed to OciDefineByPos. This would explain the > crashes that developers are experiencing with the OracleClient provider and > Sgen. > > The proposed fix is one possible solution: to allocate the values outside of > the managed heap for both the short values. > > There is another suspicious use that we should look into. The OciDefineByPos > method is actually a set of overloaded methods, with different types for the > "value" parameter.It is often a handle that is usually allocated via an > unmanaged API. But there are a few troubling uses: > ref value: used in DefineTimeStamp > ref value: when passing the reference to a Handle defined in a separate > class, DefineLob, DefineInterval > The above does not look very easy to fix. > > Given that these are resources that should be explicitly disposed, perhaps > what we should do is allocate a GCHandle for the OciDefineHandle object (from > OciStatementHandle, the one place that creates these objects) and also create > GCHandles for the objects that we use their Handle fields from (in > DefineTimeStamp, DefineLob, DefineInterval) and release the GCHandles in the > respective Dispose methods. > > Miguel ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Hello, Wanted to follow up to Neale's comment, as he clarified an important point that I overlooked. There are two ref parameters that are being passed to unmanaged code, both point to fields inside the OciDefineHandle managed type. Neale's analysis is correct: the object might move and with it, the two short variables that were passed to OciDefineByPos. This would explain the crashes that developers are experiencing with the OracleClient provider and Sgen. The proposed fix is one possible solution: to allocate the values outside of the managed heap for both the short values. There is another suspicious use that we should look into. The OciDefineByPos method is actually a set of overloaded methods, with different types for the "value" parameter.It is often a handle that is usually allocated via an unmanaged API. But there are a few troubling uses: - ref value: used in DefineTimeStamp - ref value: when passing the reference to a Handle defined in a separate class, DefineLob, DefineInterval The above does not look very easy to fix. Given that these are resources that should be explicitly disposed, perhaps what we should do is allocate a GCHandle for the OciDefineHandle object (from OciStatementHandle, the one place that creates these objects) and also create GCHandles for the objects that we use their Handle fields from (in DefineTimeStamp, DefineLob, DefineInterval) and release the GCHandles in the respective Dispose methods. Miguel On Thu, Aug 21, 2014 at 6:58 PM, Neale Ferguson wrote: > I've been looking at OciDefineHandle and the OciDefineByPos call it uses > in particular. Currently two of the parameters passed to this call are > short variables. They are passed as "ref" fields as Oracle uses their > address to put size and indicator data once the data is fetched. However, > being defined as short means that they are eligible for being moved during > garbage collection. Thus if that field is moved between the OciDefineByPos > call and the fetch of the data then what Oracle is pointing to may no > longer be variable. I'm thinking it may be better to define these fields as > IntPtr and allocate them and retrieve their values via Marshal.ReadInt16. > I'm currently testing these changes and so far I'm not getting the failures > I had been encountering. If this is a valid analysis then the OciBind > calls will also need attention as it uses a ref indp parameter as well - > these appear to be used in OracleParameter.cs. > > Neale > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] OracleClient.Oci and GC
Hey Neale, What makes a short ref suitable to be moved during GC? I am confused, I thought we wouldn't do that. Miguel On Thu, Aug 21, 2014 at 6:58 PM, Neale Ferguson wrote: > I've been looking at OciDefineHandle and the OciDefineByPos call it uses > in particular. Currently two of the parameters passed to this call are > short variables. They are passed as "ref" fields as Oracle uses their > address to put size and indicator data once the data is fetched. However, > being defined as short means that they are eligible for being moved during > garbage collection. Thus if that field is moved between the OciDefineByPos > call and the fetch of the data then what Oracle is pointing to may no > longer be variable. I'm thinking it may be better to define these fields as > IntPtr and allocate them and retrieve their values via Marshal.ReadInt16. > I'm currently testing these changes and so far I'm not getting the failures > I had been encountering. If this is a valid analysis then the OciBind > calls will also need attention as it uses a ref indp parameter as well - > these appear to be used in OracleParameter.cs. > > Neale > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] OracleClient.Oci and GC
I've been looking at OciDefineHandle and the OciDefineByPos call it uses in particular. Currently two of the parameters passed to this call are short variables. They are passed as "ref" fields as Oracle uses their address to put size and indicator data once the data is fetched. However, being defined as short means that they are eligible for being moved during garbage collection. Thus if that field is moved between the OciDefineByPos call and the fetch of the data then what Oracle is pointing to may no longer be variable. I'm thinking it may be better to define these fields as IntPtr and allocate them and retrieve their values via Marshal.ReadInt16. I'm currently testing these changes and so far I'm not getting the failures I had been encountering. If this is a valid analysis then the OciBind calls will also need attention as it uses a ref indp parameter as well - these appear to be used in OracleParameter.cs. Neale ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list