Skip to content

Conversation

arnetheduck
Copy link

Both forward declarations and methods are assumed not gcsafe and raising Exception unless otherwise specified.

This PR is a first pass over the API to ensure that it does not leak unwanted raises/not-gcsafe effects into the application using nimqml from the bulk of its API.

The changes do not touch the "public" QAIM methods but aims to fix at least the internals. Of course, raising from within a QAIM callback would crash the application but applying strict raises: [] would be a (significantly) breaking change since user code would now have to be raises-correct.

  • c functions do not raise exception (importc implies gcsafe but not raises:[].}
  • metaObject instances are not suitable to expose as a global since it's a ref type (accessing it from different threads is not gcsafe) - this PR switches to a lazy threadvar instead

Free bonus: leak fix in QModelIndex

Both forward declarations and methods are assumed not gcsafe and raising
`Exception` unless otherwise specified.

This PR is a first pass over the API to ensure that it does not leak
unwanted `raises`/not-`gcsafe` effects into the application using
`nimqml` from the bulk of its API.

The changes do not touch the "public" QAIM methods but aims to fix at
least the internals. Of course, raising from within a QAIM callback
would crash the application but applying strict `raises: []` would be a
(significantly) breaking change since user code would now have to be
raises-correct.

* `c` functions do not raise exception (`importc` implies `gcsafe` but
not `raises:[]`.}
* `metaObject` instances are not suitable to expose as a global since
it's a ref type (accessing it from different threads is not gcsafe) -
this PR switches to a lazy threadvar instead

Free bonus: leak fix in QModelIndex
@filcuc
Copy link
Owner

filcuc commented Mar 10, 2025

@arnetheduck what's the rationale for accessing the QMetaObject from different threads? Usually QObject are created inside a thread and MUST be deleted from the same thread

@arnetheduck
Copy link
Author

Usually QObject are created inside a thread and MUST be deleted from the same thread

You can create instances of the same QObject type from different threads and each instance would be deleted in "their" thread indeed - but the limitation is per instance, not per type.

The reason to do this would for example be to use signals/slots as a cross-thread communication mechanism, ie bind a signal from a QObject in one thread to a slot of an object in another thread (which is allowed).

What this PR does is bind the QMetaObject instance to a specific thread and create a new QMetaObject instance for every thread - this is slightly off because when you use qt natively, the same QMetaObject instance is used for all qobjects no matter the thread. I think this is still better than the status quo where you can't access metaobject anywhere except the main thread.

An alternative approach here would be to share the DosQMetaObject instance between threads but still use per-thread (nim) QMetaObject instances..

@filcuc
Copy link
Owner

filcuc commented Mar 15, 2025

ok but still we don't quite handle the moveToThread stuff in QObject. We can obviously add that support later on.
The same global problem is in LambdaInvoker where we keeped a global map of the lambda through QMetaObject::invokeMethod

@filcuc
Copy link
Owner

filcuc commented Mar 15, 2025

@arnetheduck i wonder if using the shared_ptr approach could be better for this use cases. In particular to the LambdaInvoker

@filcuc
Copy link
Owner

filcuc commented Mar 15, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants