[vtk-developers] Crashes after recent checkins
Berk Geveci
berk.geveci at kitware.com
Mon Aug 26 12:05:45 EDT 2013
Can you try this?
-berk
On Mon, Aug 26, 2013 at 11:28 AM, Bill Lorensen <bill.lorensen at gmail.com>wrote:
> I'm around all day.
>
>
>
> On Mon, Aug 26, 2013 at 11:25 AM, Berk Geveci <berk.geveci at kitware.com>wrote:
>
>> Hi Bill,
>>
>> If you are around today, I should have something for you shortly. If not,
>> we can commit your fix as a temporary one.
>>
>> Thanks,
>> -berk
>>
>>
>> On Mon, Aug 26, 2013 at 11:18 AM, Bill Lorensen <bill.lorensen at gmail.com>wrote:
>>
>>> Berk,
>>>
>>> I can easily reproduce the problem on two different systems. SO when you
>>> have a patch ready I can try.
>>> However, I will be offline for a few days (Tues-Thurs) this week. If you
>>> can't find a good solution quickly, perhaps we could use my patch as a
>>> temporary fix, or backout the atomic stuff until it is ready.
>>>
>>> Bill
>>>
>>>
>>>
>>> On Mon, Aug 26, 2013 at 11:10 AM, Berk Geveci <berk.geveci at kitware.com>wrote:
>>>
>>>> Bill & David: To me, the actual issue to fix is the one in
>>>> vtkTimeStamp. Both Bill's fix and what David is suggesting still hinge on
>>>> using a destroyed object, which makes me a little uncomfortable. So I think
>>>> that the right fix is one that does not access the static object in the
>>>> Modified() call. So let me think about a way of solving that problem and
>>>> bounce it around for review.
>>>>
>>>> David: The main reason I used the Internal member was to avoid
>>>> duplicating the header files in the subdirectories. Currently, they are
>>>> shared by all implementation. The alternative would be to not use
>>>> subdirectories and use the preprocessor I guess. Or to duplicate the header
>>>> files.
>>>>
>>>> -berk
>>>>
>>>>
>>>> On Mon, Aug 26, 2013 at 11:00 AM, David Gobbi <david.gobbi at gmail.com>wrote:
>>>>
>>>>> Hi Berk,
>>>>>
>>>>> The current design seems to hinge on the fact that you don't want to
>>>>> include the Kaapi/OpenMP/etc header files in vtkAtomicIntNN.h.
>>>>> But is it really such a bad thing to bring in those headers, especially
>>>>> since the default "Sequential" implementation that most people will
>>>>> use won't bring any special headers into vtkAtomicIntNN.h at all?
>>>>> It seems to me that as long as "Internal" is a pointer, there are
>>>>> going to be static initialization problems.
>>>>>
>>>>> - David
>>>>>
>>>>> > On Mon, Aug 26, 2013 at 10:16 AM, Berk Geveci <
>>>>> berk.geveci at kitware.com>
>>>>> > wrote:
>>>>> >>
>>>>> >> Thanks for tracking it btw :-)
>>>>> >>
>>>>> >>
>>>>> >> On Mon, Aug 26, 2013 at 10:15 AM, Berk Geveci <
>>>>> berk.geveci at kitware.com>
>>>>> >> wrote:
>>>>> >>>
>>>>> >>> Grrr. This is a really annoying issue with using globals like
>>>>> this. It
>>>>> >>> looks like on that machines, the order of deletion during cleanup
>>>>> is
>>>>> >>> different and causes this crash. I'll work on this today.
>>>>> >>>
>>>>> >>> -berk
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> On Sun, Aug 25, 2013 at 11:57 AM, Bill Lorensen <
>>>>> bill.lorensen at gmail.com>
>>>>> >>> wrote:
>>>>> >>>>
>>>>> >>>> Looks to me that
>>>>> >>>> static vtkAtomicInt32 GlobalTimeStamp(0)
>>>>> >>>>
>>>>> >>>> is being deleted during the finalization before some of the
>>>>> objects have
>>>>> >>>> been deleted.
>>>>> >>>>
>>>>> >>>> I verified that by printing out the this pointer in the atomic
>>>>> >>>> destructor and printing out a message if Increment() is called
>>>>> with an
>>>>> >>>> Internal = 0.
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Unpaid intern in BillsBasement at noware dot com
>>>
>>
>>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20130826/b7b927e9/attachment.html>
-------------- next part --------------
diff --git a/Common/Core/vtkTimeStamp.cxx b/Common/Core/vtkTimeStamp.cxx
index 85745ab..d29f588 100644
--- a/Common/Core/vtkTimeStamp.cxx
+++ b/Common/Core/vtkTimeStamp.cxx
@@ -18,14 +18,39 @@
#include "vtkWindows.h"
#if VTK_SIZEOF_VOID_P == 8
-# include "vtkAtomicInt64.h"
+# include "vtkAtomicInt64.h" // For global mtime
#else
-# include "vtkAtomicInt32.h"
+# include "vtkAtomicInt32.h" // For global mtime
#endif
+bool vtkTimeStamp::GlobalTimeStampFinalized = false;
+
+#if VTK_SIZEOF_VOID_P == 8
+vtkAtomicInt64* vtkTimeStamp::GlobalTimeStamp = 0;
+#else
+vtkAtomicInt32* vtkTimeStamp::GlobalTimeStamp = 0;
+#endif
+
+struct vtkTimeStampFinalizer
+{
+ inline void UseMe() {};
+
+ ~vtkTimeStampFinalizer()
+ {
+ delete vtkTimeStamp::GlobalTimeStamp;
+ vtkTimeStamp::GlobalTimeStamp = 0;
+ vtkTimeStamp::GlobalTimeStampFinalized = true;
+ }
+};
+
+static vtkTimeStampFinalizer Finalizer;
+
//-------------------------------------------------------------------------
vtkTimeStamp* vtkTimeStamp::New()
{
+ // Needed to make sure that Finalizer is created in static builds.
+ Finalizer.UseMe();
+
// If the factory was unable to create the object, then create it here.
return new vtkTimeStamp;
}
@@ -33,6 +58,12 @@ vtkTimeStamp* vtkTimeStamp::New()
//-------------------------------------------------------------------------
void vtkTimeStamp::Modified()
{
+ // GlobalTimeStamp is not longer a valid object. Return.
+ if (vtkTimeStamp::GlobalTimeStampFinalized)
+ {
+ return;
+ }
+
// Note that this would not normally be thread safe. However,
// VTK initializes static objects at load time, which in turn call
// this functions, which make this thread safe. If this behavior
@@ -42,11 +73,15 @@ void vtkTimeStamp::Modified()
// Use 32 bit atomic int on 32 bit systems, 64 bit on 64 bit systems.
// The assumption is that atomic operations will be safer when in the
// type for integer operations.
-# if VTK_SIZEOF_VOID_P == 8
- static vtkAtomicInt64 GlobalTimeStamp(0);
-# else
- static vtkAtomicInt32 GlobalTimeStamp(0);
-# endif
+ if (!vtkTimeStamp::GlobalTimeStamp)
+ {
+#if VTK_SIZEOF_VOID_P == 8
+ vtkTimeStamp::GlobalTimeStamp = new vtkAtomicInt64;
+#else
+ vtkTimeStamp::GlobalTimeStamp = new vtkAtomicInt32;
+#endif
+ }
- this->ModifiedTime = (unsigned long)GlobalTimeStamp.Increment();
+ this->ModifiedTime =
+ (unsigned long)vtkTimeStamp::GlobalTimeStamp->Increment();
}
diff --git a/Common/Core/vtkTimeStamp.h b/Common/Core/vtkTimeStamp.h
index 20a137a..55a106c 100644
--- a/Common/Core/vtkTimeStamp.h
+++ b/Common/Core/vtkTimeStamp.h
@@ -26,6 +26,9 @@
#include "vtkCommonCoreModule.h" // For export macro
#include "vtkSystemIncludes.h"
+struct vtkAtomicInt64;
+struct vtkAtomicInt32;
+
class VTKCOMMONCORE_EXPORT vtkTimeStamp
{
public:
@@ -61,6 +64,16 @@ public:
private:
unsigned long ModifiedTime;
+
+#if VTK_SIZEOF_VOID_P == 8
+ static vtkAtomicInt64* GlobalTimeStamp;
+#else
+ static vtkAtomicInt32* GlobalTimeStamp;
+#endif
+
+ static bool GlobalTimeStampFinalized;
+
+ friend class vtkTimeStampFinalizer;
};
#endif
More information about the vtk-developers
mailing list