mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 06:02:44 +00:00 
			
		
		
		
	LibJS: Do not allocate in Set's constructor
We are currently allocating in Set's constructor to create the set's underlying Map. This can cause GC to occur before the member is actually initialized, thus we will crash in Set::visit_edges trying to visit a member that does not exist. Instead, create the Map in Set::initialize, where we can allocate. Also change Map to be stored as a normal JS heap-allocated object, rather than as a stack variable.
This commit is contained in:
		
							parent
							
								
									715e56a74c
								
							
						
					
					
						commit
						c0952e3670
					
				
					 3 changed files with 16 additions and 14 deletions
				
			
		|  | @ -19,9 +19,6 @@ namespace JS { | |||
| class Map : public Object { | ||||
|     JS_OBJECT(Map, Object); | ||||
| 
 | ||||
|     // NOTE: This awkwardness is due to Set using a Map internally.
 | ||||
|     friend class Set; | ||||
| 
 | ||||
| public: | ||||
|     static Map* create(Realm&); | ||||
| 
 | ||||
|  |  | |||
|  | @ -15,14 +15,18 @@ Set* Set::create(Realm& realm) | |||
| 
 | ||||
| Set::Set(Object& prototype) | ||||
|     : Object(prototype) | ||||
|     , m_values(*prototype.shape().realm().intrinsics().map_prototype()) | ||||
| { | ||||
| } | ||||
| 
 | ||||
| void Set::initialize(Realm& realm) | ||||
| { | ||||
|     m_values = Map::create(realm); | ||||
| } | ||||
| 
 | ||||
| void Set::visit_edges(Cell::Visitor& visitor) | ||||
| { | ||||
|     Base::visit_edges(visitor); | ||||
|     static_cast<Object&>(m_values).visit_edges(visitor); | ||||
|     visitor.visit(m_values); | ||||
| } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -19,28 +19,29 @@ class Set : public Object { | |||
| public: | ||||
|     static Set* create(Realm&); | ||||
| 
 | ||||
|     virtual void initialize(Realm&) override; | ||||
|     virtual ~Set() override = default; | ||||
| 
 | ||||
|     // NOTE: Unlike what the spec says, we implement Sets using an underlying map,
 | ||||
|     //       so all the functions below do not directly implement the operations as
 | ||||
|     //       defined by the specification.
 | ||||
| 
 | ||||
|     void set_clear() { m_values.map_clear(); } | ||||
|     bool set_remove(Value const& value) { return m_values.map_remove(value); } | ||||
|     bool set_has(Value const& key) const { return m_values.map_has(key); } | ||||
|     void set_add(Value const& key) { m_values.map_set(key, js_undefined()); } | ||||
|     size_t set_size() const { return m_values.map_size(); } | ||||
|     void set_clear() { m_values->map_clear(); } | ||||
|     bool set_remove(Value const& value) { return m_values->map_remove(value); } | ||||
|     bool set_has(Value const& key) const { return m_values->map_has(key); } | ||||
|     void set_add(Value const& key) { m_values->map_set(key, js_undefined()); } | ||||
|     size_t set_size() const { return m_values->map_size(); } | ||||
| 
 | ||||
|     auto begin() const { return m_values.begin(); } | ||||
|     auto begin() { return m_values.begin(); } | ||||
|     auto end() const { return m_values.end(); } | ||||
|     auto begin() const { return const_cast<Map const&>(*m_values).begin(); } | ||||
|     auto begin() { return m_values->begin(); } | ||||
|     auto end() const { return m_values->end(); } | ||||
| 
 | ||||
| private: | ||||
|     explicit Set(Object& prototype); | ||||
| 
 | ||||
|     virtual void visit_edges(Visitor& visitor) override; | ||||
| 
 | ||||
|     Map m_values; | ||||
|     GCPtr<Map> m_values; | ||||
| }; | ||||
| 
 | ||||
| } | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Timothy Flynn
						Timothy Flynn