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-5706] DispatchTime.distantFuture does not compare with other values of DispatchTime #684

Closed
glessard opened this issue Aug 18, 2017 · 13 comments

Comments

@glessard
Copy link

glessard commented Aug 18, 2017

Previous ID SR-5706
Radar rdar://problem/34132958
Original Reporter @glessard
Type Bug
Status Closed
Resolution Done
Environment

Ubuntu, 16.04.3, Swift 4.0 snapshot 2017-08-15

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

md5: 181810e2de5e1fcdd610bb0159214367

Issue Description:

On Linux, DispatchTime.distantFuture does not fit in with the expected total ordering of DispatchTime values.

import Dispatch
let now = DispatchTime.now()
print(now == .distantFuture)
print(now > .distantFuture)
print(now < .distantFuture)

In any case, one expects one of these comparisons to be true.

On macOS, the output is (false, false, true).

On Linux, the output is (false, false, false).

@davezarzycki
Copy link

davezarzycki commented Aug 26, 2017

I think the GCD wrappers for dispatch_time_t are broken. The type is an opaque type. To understand why, one needs to look at the implementation. The dispatch_time_t type is a union of two very different notions of time:

  1. Mach absolute time, which pauses while the system is asleep and doesn't promise long-term precision.

  2. Unix "wall time", which does not pause during system sleep and promises consistent [albeit lower] precision over time.

The pausing versus not behavior makes comparing two dispatch_time_t values meaningless.

Last I knew, the encoding of the C "union" was roughly like so:

if (dt == 0) {
  // "now" -- typically used with timeout based APIs
} else if (dt == ~0) {
  // "forever" -- typically used with timeout based APIs
} else if (dt > 0) {
  // Mach absolute time -- typically used with timeout based APIs
} else {
  // Unix wall time (encoded via two or one's complement, I don't remember); probably used with a timer
}

@glessard
Copy link
Author

glessard commented Aug 26, 2017

DispatchTime and DispatchWallTime are distinct types for swift. Since it isn't possible to initialize one from a raw value, they're internally consistent.

@davezarzycki
Copy link

davezarzycki commented Aug 27, 2017

Well look at that. Interesting. FWIW – the intention of the C dispatch time API was merely as a way to feed values into GCD. It wasn't meant to be a replacement or superset of existing time APIs. I'd just look at the Swift extensions to the underlying C API as a curiosity.

@belkadan
Copy link

belkadan commented Aug 28, 2017

cc ktopley-apple (JIRA User)

@glessard
Copy link
Author

glessard commented Aug 28, 2017

@davezarzycki: There is no claim that it is "a replacement or superset of existing time APIs". However, it is convenient to use libdispatch's own formats whenever dealing directly with libdispatch – and it should work correctly.

@swift-ci
Copy link

swift-ci commented Aug 29, 2017

Comment by David Grove (JIRA)

The two copies of Time.swift in the Dispatch overlay are out of synch. The one used on Linux (swift-corelibs-libdispatch/src/swift/Time.swift) has:

public func <(a: DispatchTime, b: DispatchTime) -> Bool {
    if a.rawValue == ~0 || b.rawValue == ~0 { return false }
    return a.rawValue < b.rawValue
}

while the Darwin one (swift/stdlib/public/SDK/Dispatch/Time.swift) has

public static func < (a: DispatchTime, b: DispatchTime) -> Bool {
 return a.rawValue < b.rawValue
 }

Which matches the behavior reported by @glessard.

I think someone at Apple (ktopley-apple (JIRA User) ? ) needs to decide on the desired semantics of < on DispatchTime and then the fix for this bug is simply to get that semantics into both versions.

@swift-ci
Copy link

swift-ci commented Aug 29, 2017

Comment by David Grove (JIRA)

fwiw, I suspect the right code is actually a third variant:

public func <(a: DispatchTime, b: DispatchTime) -> Bool {
     if a.rawValue == ~0 { return false }
     if b.rawValue == ~0 { return true }
     return a.rawValue < b.rawValue
 }

@swift-ci
Copy link

swift-ci commented Aug 29, 2017

Comment by Kim Topley (JIRA)

I filed Radar 34132958 to synchronize the two versions of Time.swift.

@glessard
Copy link
Author

glessard commented Aug 29, 2017

dgrove-oss (JIRA User): that 3rd variant was exactly the patch I tried, only to see no change. I didn't realize there were two versions of the overlay!

@glessard
Copy link
Author

glessard commented Sep 8, 2017

#182 contained the fixes in apple/swift#5078 but it was abandoned without having been (fully) merged.

@swift-ci
Copy link

swift-ci commented Sep 8, 2017

Comment by Kim Topley (JIRA)

The changes to the Linux overlay are in a branch awaiting review (#298

The Darwin code is the correct version. There is no need to have special cases for ~0, because rawValue is of type dispatch_time_t, which is uint64_t and therefore the comparison "a.rawValue < b.rawValue" is unsigned.

@glessard
Copy link
Author

glessard commented Sep 8, 2017

Nice! I somehow missed this earlier. Thanks

@glessard
Copy link
Author

glessard commented Mar 5, 2019

fixed in 4.2.3 (must have been fixed before that)

@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

4 participants