Segmentation fault

Submitted by teofilis on 2008-04-15

In SVN version of WebIssues there is a bug: you get a Segmentation fault if you choose "Close connection" menu item.

The version is SVN is the same as 0.9.2. Could you provide some more details about the segfault (system configuration, stack trace)?

Regards,
Michał

I fixed this myself. What actually was happening is that every time I closed projects window (MainWindow) segmentation fault would crash the program. It is not seen at first, because it crashes at the time when program is closed. Just if you run it from console, you can see 'Segmentation fault' message. So I started a debugger.

What was happening is this: when MainWindow destructor deletes m_view (ProjectsView), QTabWidget notices this and emits tabChanged() widget, this is caught by MainWindow also and it tries to do something witch already deleted object. Therefore segmentation fault.

When 'close connection' menu item is chosen the same happens, just not in the destructor, but in MainWindow::closeView() method.

A simple fix for this. MainWindow::~MainWindow() and MainWindow::closeView() methods actually have the same piece of code: 

delete m_view;
    m_view = NULL;

    delete m_dashboard;
    m_dashboard = NULL;

So what I did is first delete a dashboard after that the view, for view to be able to handle QTabWidget's currentChanged(int) signal, and just after that to get deleted: 

delete m_dashboard;
    m_dashboard = NULL;

    delete m_view;
    m_view = NULL;

And that did solve the problem. I'm using qt-4.4.0-rc1 on Linux.

---
Also I wanted to ask a question about protocol:
Seems everyone talks about REST vs. WS-*. Why didn't you use this RESTful server backend? In such case it would be possible to GET for example http://server/webissues/issue/100 and see XML representation of an issue. Why do you use plain text in replays?

Cheers from Lithuania.

Thanks for digging into this problem. The funny thing is that on MSVC 2005 there is no crash even though tabChanged() is indeed called from the view's destructor.

Your solution works, but I think relying on the order of destruction is a bit tricky. A "cleaner" solution would be to check if both tabs still exist in the slot: 

void MainWindow::tabChanged( int index )
{
    if ( m_tabWidget->count() == 2 )
        m_view->setPageActive( index == 0 );
}

Could you please confirm that the above change also solves that problem?

Now regarding your question about the protocol. This should definitely be explained in the protocol documentation and will be as soon as I find some time to do it :).

But shortly, the WI protocol is designed strictly for the WI client and the way it works. It's meant to be simple and minimalistic, not to be a complete and general-purpose protocol for all possible clients.

Plain text doesn't have such overhead as XML; it doesn't matter for a single issue, but if you fetch a list of 1000 issues, it makes a difference. Besides the client wouldn't take any advantage from REST or XML compared to the existing protocol.

Currently Peter is working on an AJAX-based web interface for WebIssues which will probably use JSON as the protocol. Maybe one day there will be more interfaces like SOAP, REST or whatever will be the four-letter acronym of the year ;). But as long as the desktop client is concerned, it will continue to use the existing WI protocol.

Regards,
Michał

Thank you for your answers. I confirm, your patch does solve the problem.

Another minor problem on Debian is that manual (F1) location is wrong. For this to solve I modified Application::manualIndex(): instead of "/index.html" I wrote "/html/index.html", because that manual gets installed into /usr/share/doc/webissues/html/index.html. But it's probably *.deb packager's problem. I think maybe manual location could be read from configuration file, which could be placed in /etc. QSettings with SystemScope parameter can do it. This way packagers could put documentation wherever they desire.

Thanks for the info, I'll talk to the Debian packager; if it's a common standard then the code could try both paths and choose the correct one. Of course a configuration file would also be a good idea, also with path for translations (do they work correctly, btw?)

Regards,
Michał