CloudCompare / CloudCompare

CloudCompare main repository
Other
3.51k stars 1.03k forks source link

Quitting during LOD calculation causes hang #875

Open asmaloney opened 5 years ago

asmaloney commented 5 years ago

Describe the bug

If you quit CC while it is calculating LOD, it hangs waiting for a thread.

How to reproduce

  1. open a large-ish cloud (I used a ~27M point E57)
  2. rotate the view to cause the LOD to start calculating
  3. once you see output in the console like [08:21:36] [LoD] Level 3: 66 cells, quit CC before it finishes calculating

Expected behaviour

Should quit cleanly.

Additional context

Might be easier to trigger on a debug build since it's slower.

It's hanging while waiting on a thread:

void ccPointCloudLOD::clear()
{
    if (m_thread && m_thread->isRunning())
    {
        m_thread->terminate();
        m_thread->wait();
    }
...

Your environment

dgirardeau commented 5 years ago

Hum, the call order is weird here :D

We should wait for a while (like 1 second) and if the thread doesn't stop, we terminate it (violently!). But I'm not sure it explains why it crashes though...

asmaloney commented 5 years ago

Were you able to reproduce it on Windows? I know threading issues can be difficult to reproduce cross-platform.

Here's the relevant part of the stack trace when I pause it during the hang:

6  ccPointCloudLOD::clear()                       ccPointCloudLOD.cpp 522   0x1020df457    
7  ccPointCloud::clearLOD()                       ccPointCloud.cpp  5588  0x10202d987    
8  ccPointCloud::unalloactePoints()               ccPointCloud.cpp 452   0x10202d239    
9  ccPointCloud::clear()                          ccPointCloud.cpp 444   0x10202d1f9    
10 ccPointCloud::~ccPointCloud()                  ccPointCloud.cpp 433   0x10202cceb    
11 ccPointCloud::~ccPointCloud()                  ccPointCloud.cpp  432   0x10202cfb0    
12 ccPointCloud::~ccPointCloud()                  ccPointCloud.cpp 432   0x10202d0e9    
13 ccHObject::~ccHObject()                        ccHObject.cpp 94    0x101ea58c6    
14 ccHObject::~ccHObject()                        ccHObject.cpp 73    0x101ea6c55    
15 ccHObject::~ccHObject()                        ccHObject.cpp 73    0x101ea6c99    
16 ccHObject::~ccHObject()                        ccHObject.cpp 94    0x101ea58c6    
17 ccHObject::~ccHObject()                        ccHObject.cpp 73    0x101ea6c55    
18 ccHObject::~ccHObject()                        ccHObject.cpp 73    0x101ea6c99    
19 ccDBRoot::~ccDBRoot()                          ccDBRoot.cpp 321   0x100976ac2    
20 ccDBRoot::~ccDBRoot()                          ccDBRoot.cpp 318   0x100976af5    
21 ccDBRoot::~ccDBRoot()                          ccDBRoot.cpp 318   0x100976b19    
22 MainWindow::~MainWindow()                      mainwindow.cpp 350   0x100715f0b    
23 MainWindow::~MainWindow()                      mainwindow.cpp 307   0x100716f55    
24 MainWindow::~MainWindow()                      mainwindow.cpp 307   0x100716fd9    
25 MainWindow::DestroyInstance()                  mainwindow.cpp  10306 0x100819caa    
dgirardeau commented 5 years ago

Nope, I can't reproduce this issue on my side indeed

dgirardeau commented 5 years ago

Oh wait, I did get something in debug (a crash in the destructor of the ccPointCloud instance). I'll investigate

dgirardeau commented 5 years ago

Ok I fixed the issue I had (last commit: 6a4e44aea6563fda95fa225875134de6e41b8240) but I'm not sure it's directly related to this issue as I never got to the 'clearLOD' method on my side!

asmaloney commented 5 years ago

Thanks for looking into it. Unfortunately that didn't do it. Glad you found more things to fix though :-)

I'll get you the point cloud I've been using to see if there's something particular to it.

I'll try to look into this when I can set aside a bunch of time - these threading things are tough!

asmaloney commented 5 years ago

Quick followup. I'm seeing this in the console:

[Qt WARNING] [] QThread::start: Thread termination error: Operation not supported

Even after letting the LOD finish before quitting. I'm not sure what that means yet, but I'm guessing there's a problem with the way the threads are being handled...

dgirardeau commented 5 years ago

Interesting! I double checked Qt documentation, and they indeed say that one needs to wait for the 'termination' operation to finish.

And we don't have any "clean exit" mechanism (it was done quickly), this is why I had to 'terminate' the thread... So waiting without terminating won't be very useful!

Is there a reason why thread termination is not supported on macOS (or even Linux)? I know it's kind of dirty, but in this case it's pretty 'controlled'. The other (only) solution is to manage thread exit properly...

asmaloney commented 5 years ago

I'm not sure. The last time I wrote anything serious with QThread was with Qt3 :-)

According to this article (from a guy who worked extensively on QThread), we should never inherit from QThread.

dgirardeau commented 5 years ago

Yes I know this article, but it doesn't explain why you can't 'terminate' it, does it? (by the way inheriting QThread was in Qt 4 documentation :p)

There's a 'termination policy' for the threads, maybe it's disabled by default on macOS?

asmaloney commented 5 years ago

by the way inheriting QThread was in Qt 4 documentation

Yeah - I remember. That used to be the only way to do it. He accepts responsibility :-)

Using setTerminationEnabled(true) has no effect.

dgirardeau commented 5 years ago

Okay then we need to close the thread properly... but doing that during the octree computation is not possible right now... I'll have to add some modifications to the octree computation method... and add some thread safety capability to it!

dgirardeau commented 5 years ago

Or meanwhile we could display a 'wait for LOD termination' dialog with an infinite progress bar and no cancel button 💃

asmaloney commented 5 years ago

Or every time they try to quit during the calculation we put up a dialog "Stop interrupting me! I'm working here.". 🛑