Value of Objective C Future/Promise is unexpectedly nillable

[jb-gcx]

What is the problem

My understanding of the concept of djinnis Future/Promise implementation is that a future will always result in either a value of the given type or an exception. The get method waits for a result to be present and then either returns the value or throws the exception. So by the time get returns, a value must be present.

This is already the case for the Cpp implementation of Future/Promise. If I want to have a nullable result, I must use Future<std::optional<int>>.

The ObjC implementation allows calls to setValue with a nullable value, regardless of the value type. Therefore get also needs to return a nullable value. I would love to change this, because it doesn't match the implementation in other platforms and goes against my understanding of the concept. It also makes usage from Swift harder/weirder, because we always need to unwrap a value that is expected to be always present.

Possible solution

I would most like to simply change DJFuture::get and DJPromise::setValue to return and accept nonnull values.

However, I'm guessing the value was originally made nullable for a reason. Namely: what if the user wants the future to return something nullable? In C++ this is easy to achieve with Future<optional<...>>. I lack ObjC knowledge here, but I'm guessing there is no way to dictate nullability via template arg in ObjC, which is why the value was made nullable by default. Is this true? Is there no other way?

Other observations

• DJPromise::setValue without parameters sets a different kind of null than DJPromise::setValue:nil
• Marshalling between ObjC and C++ currently crashes for types that C++ expects to be non-nullable, if ObjC sets a nil value

I've created this PR in my fork to showcase these issues, see the breaking tests in the CI. For reference, I'll copy the test code and CI output here as well:

Test code

- (void)testFutureObjCNullable {
    DJPromise<NSNumber *> *p1 = [[DJPromise alloc] init];
    DJFuture<NSNumber *> *f1 = [p1 getFuture];
    [p1 setValue:nil];

    DJPromise<NSNumber *> *p2 = [[DJPromise alloc] init];
    DJFuture<NSNumber *> *f2 = [p2 getFuture];
    [p2 setValue];

    // setValue:nil doesn't set the same value as setValue without parameters.
    // Is this expected? Is it good practice to use [NSNull null], like setValue does?
    // It seems problematic to me, but I lack the ObjC knowledge.
    XCTAssertEqual([f1 get], [f2 get]);
}

- (void)testFutureRoundtripWithNil {
    // This shows a crash that occurs when passing nil into a promise that, on cpp side, doesn't expect nil values
    DJPromise<NSNumber *> *p = [[DJPromise alloc] init];
    DJFuture<NSNumber *> *f = [p getFuture];
    DJFuture<NSString *> *f2 = [DBTestHelpers futureRoundtrip:f];
    [p setValue:nil];
}

- (void)testFutureRoundtripWithNSNull {
    // This shows a crash that occurs when passing [NSNull null] into a promise that, on cpp side, doesn't expect nil values
    DJPromise<NSNumber *> *p = [[DJPromise alloc] init];
    DJFuture<NSNumber *> *f = [p getFuture];
    DJFuture<NSString *> *f2 = [DBTestHelpers futureRoundtrip:f];
    [p setValue];
}

CI output

Test Case '-[DBAsyncTests testFutureObjCNullable]' started.
/private/tmp/test-suite/handwritten-src/objc/tests/DBAsyncTest.m:66: error: -[DBAsyncTests testFutureObjCNullable] : (([f1 get]) equal to ([f2 get])) failed: ("{length = 8, bytes = 0x0000000000000000}") is not equal to ("{length = 8, bytes = 0x008c990002000000}")
Test Case '-[DBAsyncTests testFutureObjCNullable]' failed (0.726 seconds).

...

Test Case '-[DBAsyncTests testFutureRoundtripWithNil]' started.
Assertion failed: (x), function toCpp, file DJIMarshal+Private.h, line 47.

Restarting after unexpected exit, crash, or test timeout in -[DBAsyncTests testFutureRoundtripWithNil]; summary will include totals from previous launches.

...

Test Case '-[DBAsyncTests testFutureRoundtripWithNSNull]' started.
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSNull intValue]: unrecognized selector sent to instance 0x200998c00'
*** First throw call stack:
(
	0   CoreFoundation                      0x000000019401accc __exceptionPreprocess + 176
	1   libobjc.A.dylib                     0x0000000193b02788 objc_exception_throw + 60
	2   CoreFoundation                      0x00000001940cd02c -[NSObject(NSObject) __retain_OA] + 0
	3   CoreFoundation                      0x0000000193f84cdc ___forwarding___ + 1580
	4   CoreFoundation                      0x0000000193f845f0 _CF_forwarding_prep_0 + 96
	5   djinni-objc-tests                   0x0000000102899334 _ZN6djinni3I325unboxEP8NSNumber + 40
	6   djinni-objc-tests                   0x00000001028992e4 _ZN6djinni9PrimitiveINS_3I32EiE5Boxed5toCppEP8NSNumber + 108
	7   djinni-objc-tests                   0x00000001028e9b74 ___ZN6djinni13FutureAdaptorINS_3I32EE5toCppEP8DJFuture_block_invoke + 104
	8   djinni-objc-tests                   0x00000001029283d0 __17-[DJFuture then:]_block_invoke + 120
	9   djinni-objc-tests                   0x0000000102928c74 -[DJPromise updateAndCallResultHandler:] + 440
	10  djinni-objc-tests                   0x0000000102928fac -[DJPromise setValue:] + 144
	11  djinni-objc-tests                   0x00000001029290ec -[DJPromise setValue] + 72
	12  djinni-objc-tests                   0x0000000102884614 -[DBAsyncTests testFutureRoundtripWithNSNull] + 136
	13  CoreFoundation                      0x0000000193f86434 __invoking___ + 148
	14  CoreFoundation                      0x0000000193f862b4 -[NSInvocation invoke] + 428
	15  XCTestCore                          0x0000000100e8a154 +[XCTFailableInvocation invokeStandardConventionInvocation:completion:] + 68
	16  XCTestCore                          0x0000000100e8a108 __90+[XCTFailableInvocation invokeInvocation:withTestMethodConvention:lastObservedErrorIssue:]_block_invoke_3 + 28
	17  XCTestCore                          0x0000000100e89860 __81+[XCTFailableInvocation invokeWithAsynchronousWait:lastObservedErrorIssue:block:]_block_invoke + 360
	18  XCTestCore                          0x0000000100e42970 +[XCTSwiftErrorObservation observeErrorsInBlock:] + 280
	19  XCTestCore                          0x0000000100e895f0 +[XCTFailableInvocation invokeWithAsynchronousWait:lastObservedErrorIssue:block:] + 228
	20  XCTestCore                          0x0000000100e89df0 +[XCTFailableInvocation invokeInvocation:withTestMethodConvention:lastObservedErrorIssue:] + 540
	21  XCTestCore                          0x0000000100e8a1f8 +[XCTFailableInvocation invokeInvocation:lastObservedErrorIssue:] + 72
	22  XCTestCore                          0x0000000100e7b3f8 __24-[XCTestCase invokeTest]_block_invoke_2 + 88
	23  XCTestCore                          0x0000000100e51278 -[XCTMemoryChecker _assertInvalidObjectsDeallocatedAfterScope:] + 84
	24  XCTestCore                          0x0000000100e7e8d4 -[XCTestCase assertInvalidObjectsDeallocatedAfterScope:] + 92
	25  XCTestCore                          0x0000000100e7b358 __24-[XCTestCase invokeTest]_block_invoke.96 + 172
	26  XCTestCore                          0x0000000100e394b8 -[XCTestCase(XCTIssueHandling) _caughtUnhandledDeveloperExceptionPermittingControlFlowInterruptions:caughtInterruptionException:whileExecutingBlock:] + 168
	27  XCTestCore                          0x0000000100e7ae80 -[XCTestCase invokeTest] + 764
	28  XCTestCore                          0x0000000100e7c908 __26-[XCTestCase performTest:]_block_invoke.149 + 36
	29  XCTestCore                          0x0000000100e394b8 -[XCTestCase(XCTIssueHandling) _caughtUnhandledDeveloperExceptionPermittingControlFlowInterruptions:caughtInterruptionException:whileExecutingBlock:] + 168
	30  XCTestCore                          0x0000000100e7c3ac __26-[XCTestCase performTest:]_block_invoke.134 + 552
	31  XCTestCore                          0x0000000100e5d844 +[XCTContext _runInChildOfContext:forTestCase:markAsReportingBase:block:] + 180
	32  XCTestCore                          0x0000000100e5d72c +[XCTContext runInContextForTestCase:markAsReportingBase:block:] + 104
	33  XCTestCore                          0x0000000100e7be04 -[XCTestCase performTest:] + 308
	34  XCTestCore                          0x0000000100e240f0 -[XCTest runTest] + 48
	35  XCTestCore                          0x0000000100e60adc -[XCTestSuite runTestBasedOnRepetitionPolicy:testRun:] + 68
	36  XCTestCore                          0x0000000100e609a4 __27-[XCTestSuite performTest:]_block_invoke + 164
	37  XCTestCore                          0x0000000100e60408 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 48
	38  XCTestCore                          0x0000000100e5d844 +[XCTContext _runInChildOfContext:forTestCase:markAsReportingBase:block:] + 180
	39  XCTestCore                          0x0000000100e5d72c +[XCTContext runInContextForTestCase:markAsReportingBase:block:] + 104
	40  XCTestCore                          0x0000000100e60378 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 180
	41  XCTestCore                          0x0000000100e60658 -[XCTestSuite performTest:] + 220
	42  XCTestCore                          0x0000000100e240f0 -[XCTest runTest] + 48
	43  XCTestCore                          0x0000000100e60adc -[XCTestSuite runTestBasedOnRepetitionPolicy:testRun:] + 68
	44  XCTestCore                          0x0000000100e609a4 __27-[XCTestSuite performTest:]_block_invoke + 164
	45  XCTestCore                          0x0000000100e60408 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 48
	46  XCTestCore                          0x0000000100e5d844 +[XCTContext _runInChildOfContext:forTestCase:markAsReportingBase:block:] + 180
	47  XCTestCore                          0x0000000100e5d72c +[XCTContext runInContextForTestCase:markAsReportingBase:block:] + 104
	48  XCTestCore                          0x0000000100e60378 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 180
	49  XCTestCore                          0x0000000100e60658 -[XCTestSuite performTest:] + 220
	50  XCTestCore                          0x0000000100e240f0 -[XCTest runTest] + 48
	51  XCTestCore                          0x0000000100e60adc -[XCTestSuite runTestBasedOnRepetitionPolicy:testRun:] + 68
	52  XCTestCore                          0x0000000100e609a4 __27-[XCTestSuite performTest:]_block_invoke + 164
	53  XCTestCore                          0x0000000100e60408 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 48
	54  XCTestCore                          0x0000000100e5d844 +[XCTContext _runInChildOfContext:forTestCase:markAsReportingBase:block:] + 180
	55  XCTestCore                          0x0000000100e5d72c +[XCTContext runInContextForTestCase:markAsReportingBase:block:] + 104
	56  XCTestCore                          0x0000000100e60378 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 180
	57  XCTestCore                          0x0000000100e60658 -[XCTestSuite performTest:] + 220
	58  XCTestCore                          0x0000000100e240f0 -[XCTest runTest] + 48
	59  XCTestCore                          0x0000000100e26538 __89-[XCTTestRunSession executeTestsWithIdentifiers:skippingTestsWithIdentifiers:completion:]_block_invoke + 104
	60  XCTestCore                          0x0000000100e5d844 +[XCTContext _runInChildOfContext:forTestCase:markAsReportingBase:block:] + 180
	61  XCTestCore                          0x0000000100e5d72c +[XCTContext runInContextForTestCase:markAsReportingBase:block:] + 104
	62  XCTestCore                          0x0000000100e263f8 -[XCTTestRunSession executeTestsWithIdentifiers:skippingTestsWithIdentifiers:completion:] + 296
	63  XCTestCore                          0x0000000100e9e12c __72-[XCTExecutionWorker enqueueTestIdentifiersToRun:testIdentifiersToSkip:]_block_invoke_2 + 136
	64  XCTestCore                          0x0000000100e9e2c0 -[XCTExecutionWorker runWithError:] + 132
	65  XCTestCore                          0x0000000100e5a000 __25-[XCTestDriver _runTests]_block_invoke.264 + 56
	66  XCTestCore                   
libc++abi: terminating due to uncaught exception of type NSException

Restarting after unexpected exit, crash, or test timeout in -[DBAsyncTests testFutureRoundtripWithNSNull]; summary will include totals from previous launches.

[li-feng-sc]

Yes you are right, setValue 's parameter should be nonnull. For void futures, the version without parameter should be used.

[jb-gcx]

@LiFengSC I'm happy to make a PR out of this.

One more question: is there any way to prevent calling setValue without parameter for promises with non-optional values? Or to make setValue take a nullable parameter if and only if the promises value is optional? DJPromise<nonnull Foo *> doesn't compile and I'm not sure how powerful ObjC generics are in this regard 🤔

[li-feng-sc]

TBH I don't know. My understanding is ObjC generics is not as powerful as C++ templates that we can completely disable the undesirable form.