OmixVisualization / qtjambi

QtJambi is a wrapper for using Qt in Java.
http://www.qtjambi.io
Other
354 stars 40 forks source link

Crashes in NativeLink.clean when using Qt 6 #199

Closed paul35621 closed 4 months ago

paul35621 commented 5 months ago

My application works fine with Qt 5, but when I use Qt 6 it crashes when I'm using it for a while. It's not predictable when it crashes. I've had this problem for a long time, so it's not something that only happens with a recent version of Qt or QtJambi. I don't even know if it is a problem with Qt or QtJambi, since I also have some C++ code in my application, which I cannot easily remove. I'm currently using Qt 6.7.0 and QtJambi 6.7.0.

This is what the stack trace typically looks like:

Current thread (0x000001bdf75efe60):  JavaThread "QtJambiCleanupThread" daemon [_thread_in_native, id=9660, stack(0x0000008f10300000,0x0000008f10400000)]

Stack: [0x0000008f10300000,0x0000008f10400000],  sp=0x0000008f103ff100,  free space=1020k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [Qt6Gui.dll+0x2d275e]

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
J 1953  io.qt.internal.NativeUtility$NativeLink.clean(J)V (0 bytes) @ 0x000001bda097b769 [0x000001bda097b720+0x0000000000000049]
J 2480 c2 io.qt.internal.NativeUtility$NativeLink.clean()V (34 bytes) @ 0x000001bda09c29c8 [0x000001bda09c2920+0x00000000000000a8]
j  io.qt.internal.NativeUtility.lambda$static$0()V+38
j  io.qt.internal.NativeUtility$$Lambda$57+0x000001bdb701a2b8.run()V+0
j  java.lang.Thread.run()V+11 java.base@17.0.11
v  ~StubRoutines::call_stub

siginfo: EXCEPTION_ACCESS_VIOLATION (0xc0000005), reading address 0x0000000000000020

Here a few complete logs. There are some differences because of different Java versions and different ways I started the program, but they are very similar and it's always Qt6Gui.dll called from the NativeLink.clean method where it goes wrong.

Access violations are a bit out of my comfort zone. Do you have any idea what is happening here or what I could try?

omix commented 5 months ago

This looks like Java is trying to delete an object already deleted. In QtJambi, objects have a Java object and a native object. The cleanup thread deletes the native object when the Garbage Collection has deleted the Java object. Now the question is what kind of object could it be? It cannot be QObject-based type, that's for sure. And also I can exclude this object has been created in Java. It needs to be an object created internally and then made available as Java object. Did you make your C++ parts available for Java by using QtJambi generator?

paul35621 commented 5 months ago

I have a subclass of QAbstractItemModel and a subclass of QTreeView in my C++ code for which I generate Java bindings, I only create them on the Java side. I've added a println and a flush to print out the class name just before the native NativeLink.clean method is called.

The last class printed before the crash is usually io.qt.gui.QTextCursor, but also other classes such as io.qt.core.QModelIndex, io.qt.core.QRect and io.qt.widgets.QStyleOptionViewItem appear sometimes. I do use QModelIndex in my native code, I return it in my item model, but the other classes I only use from within Java.

omix commented 5 months ago

Ok but as the crash appears in QtGui it is most likely QTextCursor. How do you get a text cursor from Qt? Does the text cursor survive its text document object? I suggest explicitly calling dispose() after using the text cursor.

paul35621 commented 5 months ago

I use QTextCursor(document) to create a QTextCursor, I just call insertText and some related methods to generate a document, and then I leave it be. Yes, it seems to survive QTextDocument: Every time I get a crash when NativeLink.clean is called on a QTextCursor, I have a NativeLink.clean call on a QTextDocument right before it.

Maybe destruction of a QTextCursor fails if the QTextDocument is destructed before, but when I look at the Qt source code it looks like they anticipated on a QTextDocument being destructed before the QTextCursor:

QTextDocumentPrivate::~QTextDocumentPrivate()
{
    for (QTextCursorPrivate *curs : std::as_const(cursors))
        curs->priv = nullptr;
    cursors.clear();
QTextCursorPrivate::~QTextCursorPrivate()
{
    if (priv)
        priv->removeCursor(this);
}
void QTextDocumentPrivate::removeCursor(QTextCursorPrivate *c)
{
    cursors.remove(c);
}

I'll try the dispose thing.

paul35621 commented 5 months ago

It works! No more crashes since I dispose of QTextCursor after usage. A fix (and to know why it crashes) would still be nice though.

wolfseifert commented 5 months ago

I think that calling dispose() after usage "is" the fix.

I also had a problem with missing dispose() calls (see last comment on #155).

As far as I understand it, it comes from the difference in resource handling between C++ and Java (guaranteed destruction at the end of a block vs (now "deprecated for removal") finalize() maybe at some point in the future).

@omix:

omix commented 5 months ago

I think that calling dispose() after usage "is" the fix.

I also had a problem with missing dispose() calls (see last comment on #155).

As far as I understand it, it comes from the difference in resource handling between C++ and Java (guaranteed destruction at the end of a block vs (now "deprecated for removal") finalize() maybe at some point in the future).

@omix:

  • Is this right?
  • Do we know when it is necessary to call dispose() (besides reacting to a crash or malfunction) ?

@wolfseifert yes this is true. QtJambi has mechanisms to avoid these situations, for instance, objects are getting invalidated after sending to Java through virtual calls or objects have explicitely JavaOwnership or CppOwnership when made available for Java. These mechanisms are applied all over the QtJambi API depending on the behavior of Qt in these places. By this, objects are automatically disposed when leaving their lifetime.

The native counterpart of an object with JavaOwnership is deleted when the Java object has been deleted by GC. This is managed by the cleanup thread. However, deletion of an object is a thread-affine operation in many cases. QObject-based instances have to be deleted in the object's thread. In other cases, non-QObject objects are associated to a QObject and, by this, need to be deleted on the QObject's thread. This applies to QTextCursor which is owned by the QTextDocument. I think the crash you are observing is a race condition. At first, cleanup thread cleans QTextDocument which means deleteLater() is called and deletion takes place in main thread at any time in the future. Then, cleanup thread cleans each of the QTextCursors. Therefore, the owner of the QTextCursor is read. In parallel, this owner is deleted in main thread. Now in cleanup thread, owner is a dangling pointer. This is most likely the reason for your crash. The fields QTextDocumentPrivate::cursor and QTextCursorPrivate::priv are not protected by a mutex.

You could solve this by either disposing each text cursor after use or by explicitely disposing the text document in main thread. In this case, every text cursor is detached from its owner and deletion in a cleanup thread is safe.

I have done a lot to make text cursors safe, as I was confronted with crashes caused by cursor deletion more than a decade ago. The QtJambi legacy code was not able to delete text cursors on the document's thread. It simply caused random crashes when using text cursor all the time. I think it is not possible to make it more safe without sacrificing performance. I would have to store object associations for every text cursor object to its document. This is not applicable. I had a similar situation with QModelIndex where object associations led to inacceptable loss of performance.

@paul35621 thanks for supporting this project!

paul35621 commented 4 months ago

Thanks for the explanation, this gives me more insight in the inner working of QtJambi.

I think that calling dispose() after usage "is" the fix.

I'm happy with calling dispose() manually, but one can always forget this or simply not know about this. Undefined behaviour because of a race condition while using a single thread (as far as the QtJambi user knows) should not occur in my opinion. Random crashes are hard to debug.

Maybe the solution is to force manual disposal of a QTextCursor, with something like this in the cleanup code:

if (!object.isDisposed())
    throw new QNotDisposedException("QTextCursor objects must be disposed by calling .dispose()");

@omix You're welcome Peter, I appreciate the work you're doing!

omix commented 4 months ago

The next version of QtJambi has an option io.qt.enable-cleanup-logs which allows you to print the class and hash before a disposal is done. Additionally, I added a comment to QTextCursor and QTextDocument addressing this issue.

omix commented 4 months ago

The next version of QtJambi has an option io.qt.enable-cleanup-logs which allows you to print the class and hash before a disposal is done. Additionally, I added a comment to QTextCursor and QTextDocument addressing this issue.

The option is available in QtJambi 6.7.1 which is about to be published right now.