forked from quickshell/quickshell
		
	core/reloader: simplify generation teardown
The extra complexity previously masked the use after free in 6c95267.
			
			
This commit is contained in:
		
							parent
							
								
									84bb4098ad
								
							
						
					
					
						commit
						d56c07ceb3
					
				
					 1 changed files with 14 additions and 17 deletions
				
			
		| 
						 | 
					@ -14,7 +14,6 @@
 | 
				
			||||||
#include <qqmlcontext.h>
 | 
					#include <qqmlcontext.h>
 | 
				
			||||||
#include <qqmlengine.h>
 | 
					#include <qqmlengine.h>
 | 
				
			||||||
#include <qqmlincubator.h>
 | 
					#include <qqmlincubator.h>
 | 
				
			||||||
#include <qtimer.h>
 | 
					 | 
				
			||||||
#include <qtmetamacros.h>
 | 
					#include <qtmetamacros.h>
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#include "iconimageprovider.hpp"
 | 
					#include "iconimageprovider.hpp"
 | 
				
			||||||
| 
						 | 
					@ -47,32 +46,30 @@ EngineGeneration::EngineGeneration(const QDir& rootPath, QmlScanner scanner)
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
EngineGeneration::~EngineGeneration() {
 | 
					EngineGeneration::~EngineGeneration() {
 | 
				
			||||||
	g_generations.remove(this->engine);
 | 
						if (this->engine != nullptr) {
 | 
				
			||||||
	delete this->engine;
 | 
							qFatal() << this << "destroyed without calling destroy()";
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
void EngineGeneration::destroy() {
 | 
					void EngineGeneration::destroy() {
 | 
				
			||||||
	// Multiple generations can detect a reload at the same time.
 | 
						// Multiple generations can detect a reload at the same time.
 | 
				
			||||||
	delete this->watcher;
 | 
						QObject::disconnect(this->watcher, nullptr, this, nullptr);
 | 
				
			||||||
 | 
						this->watcher->deleteLater();
 | 
				
			||||||
	this->watcher = nullptr;
 | 
						this->watcher = nullptr;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// Yes all of this is actually necessary.
 | 
					 | 
				
			||||||
	if (this->engine != nullptr && this->root != nullptr) {
 | 
						if (this->engine != nullptr && this->root != nullptr) {
 | 
				
			||||||
		QObject::connect(this->root, &QObject::destroyed, this, [this]() {
 | 
							QObject::connect(this->root, &QObject::destroyed, this, [this]() {
 | 
				
			||||||
			// The timer seems to fix *one* of the possible qml item destructor crashes.
 | 
								// prevent further js execution between garbage collection and engine destruction.
 | 
				
			||||||
			QTimer::singleShot(0, [this]() {
 | 
								this->engine->setInterrupted(true);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
								g_generations.remove(this->engine);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
			// Garbage is not collected during engine destruction.
 | 
								// Garbage is not collected during engine destruction.
 | 
				
			||||||
			this->engine->collectGarbage();
 | 
								this->engine->collectGarbage();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
				QObject::connect(this->engine, &QObject::destroyed, this, [this]() { delete this; });
 | 
								delete this->engine;
 | 
				
			||||||
 | 
					 | 
				
			||||||
				// Even after all of that there's still multiple failing assertions and segfaults.
 | 
					 | 
				
			||||||
				// Pray you don't hit one.
 | 
					 | 
				
			||||||
				// Note: it appeats *some* of the crashes are related to values owned by the generation.
 | 
					 | 
				
			||||||
				// Test by commenting the connect() above.
 | 
					 | 
				
			||||||
				this->engine->deleteLater();
 | 
					 | 
				
			||||||
			this->engine = nullptr;
 | 
								this->engine = nullptr;
 | 
				
			||||||
			});
 | 
								delete this;
 | 
				
			||||||
		});
 | 
							});
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		this->root->deleteLater();
 | 
							this->root->deleteLater();
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue