Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On Wednesday 20 January 2016 12:34:33 Mojmír Svoboda wrote: > On 20.1.2016 11:26, Igor Mironchik wrote: > > Just looked a little at your code. And that what I can say: your code > > has nothing with QStringLiteral. > > QStringDataTable uses the same technique as QStringLiteral. There's no such class. Don't name your classes with Q + capital letter, please. > > And I don't understand why you need such a "crazy" :) code? May be would > > be much more better and effective to implement custom model and just > > store in the model normal QStrings? > > Normally these strings are read-only so there is no reason to allocate > them on a heap. And there are lots of them (may exceed order 10^6) > Then I met the article I mentioned earlier so I went for it, because it > seems like a neat feature... what could possibly go wrong? :) Just don't unmap your file. Ever. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 20.1.2016 11:26, Igor Mironchik wrote: Just looked a little at your code. And that what I can say: your code has nothing with QStringLiteral. QStringDataTable uses the same technique as QStringLiteral. And I don't understand why you need such a "crazy" :) code? May be would be much more better and effective to implement custom model and just store in the model normal QStrings? Normally these strings are read-only so there is no reason to allocate them on a heap. And there are lots of them (may exceed order 10^6) Then I met the article I mentioned earlier so I went for it, because it seems like a neat feature... what could possibly go wrong? :) And you forgot to delete selection model! Seems that this cache of QString exists because selection model use it?! oh yeah, i indeed forgot. will try. cheers, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
Hi, On 20.01.2016 12:45, Mojmír Svoboda wrote: On 19.1.2016 10:37, Giuseppe D'Angelo wrote: Please trim down to a small testcase and submit a bug report. If your model is well-behaving the view can't cache the option that way. I managed to reproduce the behaviour with QStandardItemModel, so here is the repro case: https://bugreports.qt.io/browse/QTBUG-50551 Just looked a little at your code. And that what I can say: your code has nothing with QStringLiteral. And I don't understand why you need such a "crazy" :) code? May be would be much more better and effective to implement custom model and just store in the model normal QStrings? And you forgot to delete selection model! Seems that this cache of QString exists because selection model use it?! Many thanks guys, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 20.1.2016 11:38, Igor Mironchik wrote: Why are you deleting model? i am trying to remove everything i can so that there are no references to the underlying data (m_strings). Just implement clear() method and between beginResetModel(), endResetModel() delete your data. And I guess that this crash will not occur again. not sure if it will help, can try. but the value is cached in QCommonStyle, not the model. Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 20.01.2016 14:25, Mojmír Svoboda wrote: On 20.1.2016 11:38, Igor Mironchik wrote: Why are you deleting model? i am trying to remove everything i can so that there are no references to the underlying data (m_strings). Just implement clear() method and between beginResetModel(), endResetModel() delete your data. And I guess that this crash will not occur again. not sure if it will help, can try. but the value is cached in QCommonStyle, not the model. When you call beginResetModel() and endResetModel() view clears everything cached... Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 19.1.2016 10:37, Giuseppe D'Angelo wrote: Please trim down to a small testcase and submit a bug report. If your model is well-behaving the view can't cache the option that way. I managed to reproduce the behaviour with QStandardItemModel, so here is the repro case: https://bugreports.qt.io/browse/QTBUG-50551 Many thanks guys, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 20.01.2016 12:45, Mojmír Svoboda wrote: On 19.1.2016 10:37, Giuseppe D'Angelo wrote: Please trim down to a small testcase and submit a bug report. If your model is well-behaving the view can't cache the option that way. I managed to reproduce the behaviour with QStandardItemModel, so here is the repro case: https://bugreports.qt.io/browse/QTBUG-50551 Why are you deleting model? Just implement clear() method and between beginResetModel(), endResetModel() delete your data. And I guess that this crash will not occur again. Many thanks guys, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 20.01.2016 14:34, Mojmír Svoboda wrote: On 20.1.2016 11:26, Igor Mironchik wrote: Just looked a little at your code. And that what I can say: your code has nothing with QStringLiteral. QStringDataTable uses the same technique as QStringLiteral. QStringLiteral uses static structs! You don't. And I don't understand why you need such a "crazy" :) code? May be would be much more better and effective to implement custom model and just store in the model normal QStrings? Normally these strings are read-only so there is no reason to allocate them on a heap. And there are lots of them (may exceed order 10^6) Then I met the article I mentioned earlier so I went for it, because it seems like a neat feature... what could possibly go wrong? :) I do not know. I need benchmarks for it. May be you will win nothing... And you forgot to delete selection model! Seems that this cache of QString exists because selection model use it?! oh yeah, i indeed forgot. will try. cheers, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 18.1.2016 14:48, Igor Mironchik wrote: What if to change QStringLiteral with QLatin1String usage? Tried it yesterday, and it works without problems as the reference count in QStringData ensures lifetime of the QString. I also tried some very dirty way to release the cachedOption before closing the widget and it worked on a sample. Worth a try, but not a solution :) Nice day, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On Mon, Jan 18, 2016 at 9:40 AM, Mojmír Svobodawrote: > One of the copies is in QCommonStylePrivate::cachedOption::text field. I > suspect these style classes use some form of sharing, so when i close the > widget, the copy of qstring will survive the storage, and then when new > qtableview opens, it tries to reuse the style and on the first use it > destroys the old content of .text field and crash follows. > That is correct. It's an unfortunate side effect of QStrings built like QStringLiteral does. We've seen that already when unloading plugins which have handed out QStrings... > Now what do i do? Try to force somehow the style class to drop the > d->cachedOption? > Any ideas, please? > Please trim down to a small testcase and submit a bug report. If your model is well-behaving the view can't cache the option that way. You would have crashes even if you use entirely public APIs (such as QString::fromRawData) and that's unacceptable. HTH, -- Giuseppe D'Angelo ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 18.01.2016 13:46, Mojmír Svoboda wrote: On 18.1.2016 10:21, Igor Mironchik wrote: Can you reset model on closing of your table view, drop any data, etc... And then close view. Yes i am doing that. I am trying to close everything i know i created (models, proxy models, context menus, delegates). But the problem persists. What if to change QStringLiteral with QLatin1String usage? While inpecting the code i came to think that this class (QWindowsVistaStyle in current case) is not deleted when i close my table, but rather shared between multiple widgets. If i take QStyle * s0 = m_main_window->dockManager().style(); for example where dockManager() is an unrelated QTreeView, the .text field in this s0 contains .text from completely different QTableView (the one that i am closing). If I understood your correctly you are loading model again on new show? i have one widget right from start, this is the dockManager with the style s0. This widget is the first to create and therefore i think it's the first to create the style via QStyleFactory. Some (all?) subsequent views seems to re-use this s0. And then there is this QTableView that opens when tcp connection with data comes and closed on new connection with new data or user request. Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
[Interest] invalidated copies of QString in QCommonStyle::cachedOption
Hello, i have a crash in QCommonStyle::subElementRect case SE_ItemViewItemDecoration: case SE_ItemViewItemText: case SE_ItemViewItemFocusRect: if (const QStyleOptionViewItem *vopt = ...) { if (!d->isViewItemCached(*vopt)) { d->viewItemLayout(...); if (d->cachedOption) { delete d->cachedOption; d->cachedOption = 0; } d->cachedOption = new QStyleOptionViewItem(*vopt); } with callstack: QArrayData::deallocate QTypedArrayData::deallocate(QArrayData * data) QString::~QString() Line 1070 QStyleOptionViewItem::~QStyleOptionViewItem() QStyleOptionViewItem::`scalar deleting destructor'(unsigned int) > QCommonStyle::subElementRect(QStyle::SubElement sr, const QStyleOption * opt, const QWidget * widget) Line 2993 QWindowsStyle::subElementRect QWindowsVistaStyle::subElementRect QCommonStyle::drawControl QWindowsStyle::drawControl QWindowsVistaStyle::drawControl QStyledItemDelegate::paint QTableViewPrivate::drawCell QTableView::paintEvent ... How this occurs: i have QTableView with custom model that uses memmapped file filled with lots of QStringData. These QStringData are set using the technique presented in http://woboq.com/blog/qstringliteral.html for example. The problem is when the storage goes away (widget is closed), because there may be copies of those strings, and indeed they are. One of the copies is in QCommonStylePrivate::cachedOption::text field. I suspect these style classes use some form of sharing, so when i close the widget, the copy of qstring will survive the storage, and then when new qtableview opens, it tries to reuse the style and on the first use it destroys the old content of .text field and crash follows. Now what do i do? Try to force somehow the style class to drop the d->cachedOption? Any ideas, please? Best regards, Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest
Re: [Interest] invalidated copies of QString in QCommonStyle::cachedOption
On 18.1.2016 10:21, Igor Mironchik wrote: Can you reset model on closing of your table view, drop any data, etc... And then close view. Yes i am doing that. I am trying to close everything i know i created (models, proxy models, context menus, delegates). But the problem persists. While inpecting the code i came to think that this class (QWindowsVistaStyle in current case) is not deleted when i close my table, but rather shared between multiple widgets. If i take QStyle * s0 = m_main_window->dockManager().style(); for example where dockManager() is an unrelated QTreeView, the .text field in this s0 contains .text from completely different QTableView (the one that i am closing). If I understood your correctly you are loading model again on new show? i have one widget right from start, this is the dockManager with the style s0. This widget is the first to create and therefore i think it's the first to create the style via QStyleFactory. Some (all?) subsequent views seems to re-use this s0. And then there is this QTableView that opens when tcp connection with data comes and closed on new connection with new data or user request. Mojmir ___ Interest mailing list Interest@qt-project.org http://lists.qt-project.org/mailman/listinfo/interest