Ignore:
Timestamp:
Aug 5, 2007, 3:16:41 AM (18 years ago)
Author:
darin
Message:

Reviewed by Maciej.

  • fix <rdar://problem/5371862> crash in Dashcode due to Quartz Composer JavaScript garbage collector reentrancy
  • API/JSBase.cpp: (JSGarbageCollect): Don't call collector() if isBusy() returns true.
  • kjs/collector.h: Added isBusy(), removed the unused return value from collect()
  • kjs/collector.cpp: Added an "operation in progress" flag to the allocator. (KJS::Collector::allocate): Call abort() if an operation is already in progress. Set the new flag instead of using the debug-only GCLock. (KJS::Collector::collect): Ditto. (KJS::Collector::isBusy): Added.
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/kjs/collector.cpp

    r24843 r24874  
    22/*
    33 *  This file is part of the KDE libraries
    4  *  Copyright (C) 2003, 2004, 2005, 2006 Apple Computer, Inc.
     4 *  Copyright (C) 2003, 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    55 *
    66 *  This library is free software; you can redistribute it and/or
    … …  
    2323#include "collector.h"
    2424
     25#include "internal.h"
     26#include "list.h"
     27#include "value.h"
     28#include <algorithm>
     29#include <setjmp.h>
     30#include <stdlib.h>
    2531#include <wtf/FastMalloc.h>
    2632#include <wtf/FastMallocInternal.h>
    2733#include <wtf/HashCountedSet.h>
    2834#include <wtf/UnusedParam.h>
    29 #include "internal.h"
    30 #include "list.h"
    31 #include "value.h"
    32 
    33 #include <setjmp.h>
    34 #include <algorithm>
    3535
    3636#if USE(MULTIPLE_THREADS)
    … …  
    6969namespace KJS {
    7070
    71 
    7271// tunable parameters
    7372
    … …  
    7877const size_t ALLOCATIONS_PER_COLLECTION = 4000;
    7978
     79enum OperationInProgress { NoOperation, Allocation, Collection };
     80
    8081struct CollectorHeap {
    81   CollectorBlock **blocks;
     82  CollectorBlock** blocks;
    8283  size_t numBlocks;
    8384  size_t usedBlocks;
    … …  
    8788  size_t numLiveObjectsAtLastCollect;
    8889  size_t extraCost;
     90
     91  OperationInProgress operationInProgress;
    8992};
    9093
    91 static CollectorHeap heap = {NULL, 0, 0, 0, 0, 0, 0};
    92 
     94static CollectorHeap heap = { 0, 0, 0, 0, 0, 0, 0, NoOperation };
     95
     96// FIXME: I don't think this needs to be a static data member of the Collector class.
     97// Just a private global like "heap" above would be fine.
    9398size_t Collector::mainThreadOnlyObjectCount = 0;
     99
    94100bool Collector::memoryFull = false;
    95 
    96 #ifndef NDEBUG
    97 
    98 class GCLock {
    99     static bool isLocked;
    100 
    101 public:
    102     GCLock()
    103     {
    104         ASSERT(!isLocked);
    105         isLocked = true;
    106     }
    107    
    108     ~GCLock()
    109     {
    110         ASSERT(isLocked);
    111         isLocked = false;
    112     }
    113 };
    114 
    115 bool GCLock::isLocked = false;
    116 #endif
    117101
    118102static CollectorBlock* allocateBlock()
    … …  
    191175  UNUSED_PARAM(s); // s is now only used for the above assert
    192176
     177  ASSERT(heap.operationInProgress == NoOperation);
     178  // FIXME: If another global variable access here doesn't hurt performance
     179  // too much, we could abort() in NDEBUG builds, which could help ensure we
     180  // don't spend any time debugging cases where we allocate inside an object's
     181  // deallocation code.
     182
    193183  // collect if needed
    194184  size_t numLiveObjects = heap.numLiveObjects;
    … …  
    202192  }
    203193 
     194  ASSERT(heap.operationInProgress == NoOperation);
    204195#ifndef NDEBUG
    205   GCLock lock;
     196  // FIXME: Consider doing this in NDEBUG builds too (see comment above).
     197  heap.operationInProgress = Allocation;
    206198#endif
    207199 
    … …  
    252244  targetBlock->usedCells = static_cast<uint32_t>(targetBlockUsedCells + 1);
    253245  heap.numLiveObjects = numLiveObjects + 1;
     246
     247#ifndef NDEBUG
     248  // FIXME: Consider doing this in NDEBUG builds too (see comment above).
     249  heap.operationInProgress = NoOperation;
     250#endif
    254251
    255252  return newCell;
    … …  
    760757  ASSERT(JSLock::currentThreadIsHoldingLock());
    761758
    762 
    763 #ifndef NDEBUG
    764   GCLock lock;
    765 #endif
    766  
    767 #if USE(MULTIPLE_THREADS)
    768     bool currentThreadIsMainThread = onMainThread();
    769 #else
    770     bool currentThreadIsMainThread = true;
    771 #endif
    772    
     759  ASSERT(heap.operationInProgress == NoOperation);
     760  if (heap.operationInProgress != NoOperation)
     761    abort();
     762
     763  heap.operationInProgress = Collection;
     764
     765  bool currentThreadIsMainThread = onMainThread();
     766
    773767  // MARK: first mark all referenced objects recursively starting out from the set of root objects
    774768
    … …  
    901895  memoryFull = (numLiveObjects >= KJS_MEM_LIMIT);
    902896
     897  heap.operationInProgress = NoOperation;
     898
    903899  return deleted;
    904900}
    … …  
    908904  return heap.numLiveObjects;
    909905}
    910 
    911 #ifdef KJS_DEBUG_MEM
    912 void Collector::finalCheck()
    913 {
    914 }
    915 #endif
    916906
    917907size_t Collector::numInterpreters()
    … …  
    978968}
    979969
     970bool Collector::isBusy()
     971{
     972    return heap.operationInProgress != NoOperation;
     973}
     974
    980975} // namespace KJS
Note: See TracChangeset for help on using the changeset viewer.