Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-737] [dispatch] dispatch_queue_create has wrong return type #755

Closed
drewcrawford opened this issue Feb 15, 2016 · 11 comments
Closed

[SR-737] [dispatch] dispatch_queue_create has wrong return type #755

drewcrawford opened this issue Feb 15, 2016 · 11 comments

Comments

@drewcrawford
Copy link
Contributor

Previous ID SR-737
Radar None
Original Reporter @drewcrawford
Type Bug
Status Resolved
Resolution Won't Do
Environment

Linux x64
swift-DEVELOPMENT-SNAPSHOT-2016-02-08-a

Additional Detail from JIRA
Votes 0
Component/s libdispatch
Labels Bug
Assignee dgrove-oss (JIRA)
Priority Medium

md5: e7691fb73226e77e3cd6b0d9ebce19d7

Issue Description:

dispatch_queue_create on Linux has return type of COpaquePointer whereas a return type of dispatch_queue_t! is expected.

To reproduce in my Docker image:

$ docker run -it --privileged=true drewcrawford/swift:swift-DEVELOPMENT-SNAPSHOT-2016-02-08-a
$ swift -Xcc -fblocks -L/usr/local/lib -I /usr/local/include/dispatch/haxx -lBlocksRuntime -L/usr/lib/x86_64-linux-gnu
Welcome to Swift version 3.0-dev (LLVM a7663bb722, Clang 4ca3c7fa28, Swift c130b422a9). Type :help for assistance.
  1> import Dispatch
  2> let q = dispatch_queue_create("foo", DISPATCH_QUEUE_SERIAL)
q: dispatch_queue_t = 0x0000000000603050 -> 0x00007ffff442a6a0 libdispatch.so`_dispatch_queue_vtable
  3> dispatch_suspend(q)

Expected results: success

Actual results:

repl.swift:3:18: error: cannot convert value of type 'dispatch_queue_t' (aka 'COpaquePointer') to expected argument type 'dispatch_object_t'
dispatch_suspend(q)
@drewcrawford
Copy link
Contributor Author

cc: dgrove-oss (JIRA User)

@swift-ci
Copy link

Comment by David Grove (JIRA)

This is a general problem on Linux right now because of the lack of objective C. The importer appears to not understand the transparent union type dispatch_object_t, so none of the API functions that take dispatch_object_t currently work from Swift. Need to figure out if it is practical to get the importer to understand the transparent union, or if we need to add a whole slew of wrapper functions to do the translations.

@belkadan
Copy link

It's not specifically the transparent_union that's the problem, but the fact that none of these types have definitions. Swift currently doesn't have a good way to represent a pointer to a forward-declared struct/union/enum; it doesn't just pretend they're empty because you might import the definition later on in your REPL session. We definitely need a better answer than that, though.

@swift-ci
Copy link

Comment by David Grove (JIRA)

Thanks for the correction Jordan.

I did some quick greps to try to scope the problem. There are fewer API functions that use dispatch_object_t than I'd guessed before looking. All of these functions only take 1 parameter of type dispatch_object_t (so no combinatorial explosion). So perhaps working around it via some overloaded functions in the Linux overlay isn't totally impractical. In the public .h files, dispatch_object_t only shows up in introspection.h (3 functions: dispatch_introspection_hook_queue_item_enqueue, dispatch_introspection_hook_queue_item_dequeue, dispatch_introspection_hook_queue_item_complete) queue.h (1 function: dispatch_set_target_queue), and object.h (8 non-deprecated functions: dispatch_retain, dispatch_release, dispatch_get_context, dispatch_set_context, dispatch_set_finalizer_f, dispatch_suspend, dispatch_resume, dispatch_notify).

@belkadan Do you think working around by defining a bunch of wrapper functions in the overlay is the way to tackle it (at least for now), or do you have another suggestion?

thanks!

@swift-ci
Copy link

Comment by David Grove (JIRA)

I took a cut at fixing this by adding some overloaded functions to Dispatch.swift backed by C functions in swift_wrappers.c It almost works, but there is something I'm not understanding about the dialect of Swift that needs to be used in Dispatch.swift or in how the types are being imported. In particular, I would like to write a set of overloaded functions like:

public func dispatch_resume(obj:dispatch_queue_t) {
  _swift_dispatch_resume_for_queue(obj)
}
public func dispatch_resume(obj:dispatch_source_t) {
  _swift_dispatch_resume_for_source(obj)
}

If I import Dispatch to get dispatch_queue_t and dispatch_object_t, then swiftc rejects the code saying:

/home/vagrant/swift-corelibs-libdispatch/src/swift/Dispatch.swift:92:13: error: invalid redeclaration of 'dispatch_resume'
public func dispatch_resume(obj:dispatch_source_t) {
            ^
/home/vagrant/swift-corelibs-libdispatch/src/swift/Dispatch.swift:86:13: note: 'dispatch_resume' previously declared here
public func dispatch_resume(obj:dispatch_queue_t) {
            ^

If I don't import Dispatch and provide dummy definitions

public class dispatch_queue_t { }
public class dispatch_source_t { }

then the overloaded functions compile just fine (no complaint about redeclaring dispatch_resume).

Is the problem that we can't overload on COpaquePointer (no matter what name it was given)? Is there some annotation or pragma that would help?

Note that if I don't overload (for example, just define for dispatch_queue_t), then libdispatch builds successfully and programs linked against it can call dispatch_suspend and dispatch_resume on a dispatch_queue_t.

Code is sitting on a branch if looking at it helps suggest an approach I should try: https://github.com/dgrove-oss/swift-corelibs-libdispatch/tree/SR-737

@drewcrawford
Copy link
Contributor Author

Right now on Linux, dispatch_queue_t and dispatch_source_t are all COpaquePointer (e.g., they are merely a typealias for it). In Swift, typealiases are erased very early on in the parse. I don't know the exact motivation for that "feature", but I suspect it simplifies implicit casting between typealiases, which is not normally allowed for arbitrary Swift types.

...since the typealias was erased, the function signature becomes dispatch_resume(obj:COpaquePointer) in both cases. The compiler complains about a redeclaration because the "check duplicates" pass comes after the pass to erase the typealias.

You're correct that when you declare e.g. dispatch_queue_t to be "honest-to-god types" (that are not aliased to each other) we are no longer in that situation and so it will compile.

Whether or not it will run is a different story. I expect something bad will happen if you try to bridge (e.g., via pointer) a Swift object and whatever the implementation of dispatch_object_t on Linux is, because Swift will try to reference count it etc. On Darwin, that implementation bridges to Objective-C, and the Swift instance comes in from the ObjC bridge, but there's no Objective-C here, so I dunno what kind of a "thing" that C API is on Linux, but it's probably not something the Swift runtime understands how to refcount.

My suggestion would be to try and vend our own wrapper class in Swift-land, along the lines of

class dispatch_object_t {
    private var linuxImplementation: COpaquePointer
    dealloc() {
        dispatch_release(linuxImplementation)
    }
}
class dispatch_queue_t : dispatch_object_t { }

func dispatch_resume(obj: dispatch_queue_t) {
       _underlying_c_libdispatch_resume(obj.linuxImplementation)
}

etc.

The big idea here is that a "real" Swift class would be refcountable by the Swift runtime, so it would behave to clients exactly as they expect from Darwin/ObjC. However, we would have to wrap the dispatch_functions as well, to pull out the COpaquePointer from inside the wrapper. I think something along these lines would cleanly solve SR-740 as well.

One thing I don't know is how to make sure clients see the wrapper functions, and not the C functions being wrapped. I assume there's an annotation for this, but I don't actually know.

@belkadan
Copy link

Context for COpaquePointer: if you're in the REPL, you might import one module that has an incomplete struct type, do something with it, and then import another module that has the definition. We definitely don't want to be updating existing types in your session, so we had to pick a different type.

Replacing COpaquePointer with some kind of OpaquePointer<T> (or just UnsafePointer<T> with a zero-sized type, which shouldn't be able to do any harm) is definitely something to look into, but we'd need to be careful how it interacts with the REPL. Literally every other context (libraries, scripts, playgrounds) is compiled all at once, though, so the REPL solution doesn't have to be perfect.

@swift-ci
Copy link

Comment by David Grove (JIRA)

I'll explore some options next week. I think we want a final solution that doesn't require wrapping for efficiency. We have complete control over the "shape" of the C dispatch objects on Linux, so we should be able to give them headers that are compatible with Swift objects. Right now they are given headers that are compatible with objective C (even though they are not objective C objects). Similarly, we can redirect dispatch_retain/release functions to the Swift impls.

@belkadan, where would I look to find the canonical definition of a Swift object header? Ideally there would be a .h file somewhere I could include into libdispatch when building it with Swift support (libdispatch has an autoconf flag for Swift already that could be used to guard the include).

@drewcrawford
Copy link
Contributor Author

That's a fascinating approach. Unfortunately I don't know enough about Swift internals to be a lot of help here.

I do have some other unrelated cases where I'm interested in "spoofing" Swift objects, so I am fascinated to follow what goes on here.

@swift-ci
Copy link

Comment by David Grove (JIRA)

After quite a bit of exploring alternatives, I decided that changing libdispatch to implement the Swift object model was too invasive a change to libdispatch. So, I went back to the straightforward vending approach and submitted #61 which fixes this issue.

@swift-ci
Copy link

Comment by David Grove (JIRA)

made obsolete by Swift 3 dispatch overlay changes.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants