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

Failsafe to ensure obj is not nil when creating dict literal #218

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Failsafe to ensure obj is not nil when creating dict literal #218

merged 1 commit into from
Aug 4, 2022

Conversation

okalentiev
Copy link
Contributor

Description of the change

This PR adds a defensive failsafe to ensure that object is not nil while serializing it. In case of nil, Foundation throws the following exception: -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[0].

My first assumption was that this might happen due to implicit coercing of Swift's optional values to the Any type. For this, I have added a test case with a nil optional string. However, this worked as expected, resulting in a JSON with a null values added and no exception thrown.

To reproduce the error, I hardcoded obj to nil in the following way:

    [obj enumerateKeysAndObjectsUsingBlock:^(id  _Nonnull key, id  _Nonnull obj, BOOL * _Nonnull stop) {
        // Hardcoded here
        obj = nil;
        
        if ([obj isKindOfClass:[NSDictionary class]]) {
            
            [safeData setObject:[[self class] rollbar_safeDataFromJSONObject:obj] forKey:key];
        } else if ([obj isKindOfClass:[NSArray class]]) {
            
            [safeData setObject:((NSArray *)obj).mutableCopy forKey:key];
        } else if ([NSJSONSerialization isValidJSONObject:@{key:obj}]) {

Then I could get an exception. Additionally I also tried to play with different objects and dictionaries containing nil values, but could not trigger exceptions.

This change might not be conceptually correct since the nil value's root cause is unclear. However, a defensive fix is a small price for not crashing the app.

Thinking out loud: Considering that the exception is not happening every time, the issue might suggest some race condition issues. Dictionaries are not thread safe, and if the dictionary is modified during enumeration, this could cause this exception.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Maintenance
  • New release

Related issues

Checklists

Development

  • I could not figure out how to run linting
  • I'm not sure how to simulate the use case where the dictionary contains nil objects, so I only covered existing use case with a test
  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers assigned
  • Issue from task tracker has a link to this pull request
  • Changes have been reviewed by at least one other engineer

@akornich akornich self-requested a review August 4, 2022 19:49
@akornich akornich self-assigned this Aug 4, 2022
@akornich akornich added bug Something isn't working RollbarNotifier RollbarNotifier mosule specific labels Aug 4, 2022
@akornich akornich added RollbarCommon RollbarCommon module specific and removed RollbarNotifier RollbarNotifier mosule specific labels Aug 4, 2022
Copy link
Contributor

@akornich akornich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a very useful last-chance defense. Thank you, @okalentiev !

@akornich akornich merged commit ff4acf3 into rollbar:master Aug 4, 2022
This was referenced Aug 4, 2022
@okalentiev okalentiev deleted the bugfix/215-json-serialization-crash branch August 5, 2022 07:32
@freemansion
Copy link

We're currently experiencing crash due the same problem.

Fatal Exception: NSInvalidArgumentException
*** +[NSJSONSerialization dataWithJSONObject:options:error:]: value parameter is nil

Can someone please have a look.
Here is more context:

46 crashes / 18 users
Operating systems
65% iOS 16
35% iOS 17

Fatal Exception: NSInvalidArgumentException
0  CoreFoundation                 0x9e88 __exceptionPreprocess
1  libobjc.A.dylib                0x178d8 objc_exception_throw
2  Foundation                     0x51140 +[NSData(NSData) dataWithBytesNoCopy:length:freeWhenDone:]
3  RollbarCommon                  0x43c0 +[NSJSONSerialization(Rollbar) rollbar_dataWithJSONObject:options:error:safe:] + 43 (NSJSONSerialization+Rollbar.m:43)
4  RollbarNotifier                0x14b00 -[RollbarSession saveSessionState:] + 488 (RollbarSession.m:488)
5  RollbarNotifier                0x13c9c -[RollbarSession enableOomMonitoring:withCrashCheck:] + 127 (RollbarSession.m:127)
6  RollbarNotifier                0xa924 -[RollbarInfrastructure configureWith:andCrashCollector:] + 76 (RollbarInfrastructure.m:76)
7  RollbarNotifier                0x4110 +[Rollbar initWithAccessToken:configuration:crashCollector:] + 79 (Rollbar.m:79)
8  App Name                       0x286458 RollbarHandler.initRollbarWithUser(phone:userName:) + 49 (RollbarHandler.swift:49)
9  App Name                       0x421df8 AppDelegate.application(_:didFinishLaunchingWithOptions:) + 80 (AppDelegate.swift:80)
10 App Name                       0x4282b0 @objc AppDelegate.application(_:didFinishLaunchingWithOptions:) (<compiler-generated>)
11 UIKitCore                      0x35a888 -[UIApplication _handleDelegateCallbacksWithOptions:isSuspended:restoreState:]
12 UIKitCore                      0x359fac -[UIApplication _callInitializationDelegatesWithActions:forCanvas:payload:fromOriginatingProcess:]
13 UIKitCore                      0x358f88 -[UIApplication _runWithMainScene:transitionContext:completion:]
14 UIKitCore                      0x358bd4 -[_UISceneLifecycleMultiplexer completeApplicationLaunchWithFBSScene:transitionContext:]
15 UIKitCore                      0x9e600 _UIScenePerformActionsWithLifecycleActionMask
16 UIKitCore                      0x3e0918 __101-[_UISceneLifecycleMultiplexer _evalTransitionToSettings:fromSettings:forceExit:withTransitionStore:]_block_invoke
17 UIKitCore                      0x291014 -[_UISceneLifecycleMultiplexer _performBlock:withApplicationOfDeactivationReasons:fromReasons:]
18 UIKitCore                      0x290dcc -[_UISceneLifecycleMultiplexer _evalTransitionToSettings:fromSettings:forceExit:withTransitionStore:]
19 UIKitCore                      0x29097c -[_UISceneLifecycleMultiplexer uiScene:transitionedFromState:withTransitionContext:]
20 UIKitCore                      0x290848 __186-[_UIWindowSceneFBSSceneTransitionContextDrivenLifecycleSettingsDiffAction _performActionsForUIScene:withUpdatedFBSScene:settingsDiff:fromSettings:transitionContext:lifecycleActionType:]_block_invoke
21 UIKitCore                      0x972fa8 +[BSAnimationSettings(UIKit) tryAnimatingWithSettings:fromCurrentState:actions:completion:]
22 UIKitCore                      0xa0bf98 _UISceneSettingsDiffActionPerformChangesWithTransitionContextAndCompletion
23 UIKitCore                      0x13c958 -[_UIWindowSceneFBSSceneTransitionContextDrivenLifecycleSettingsDiffAction _performActionsForUIScene:withUpdatedFBSScene:settingsDiff:fromSettings:transitionContext:lifecycleActionType:]
24 UIKitCore                      0x5af7a8 __64-[UIScene scene:didUpdateWithDiff:transitionContext:completion:]_block_invoke.214
25 UIKitCore                      0x20f0b8 -[UIScene _emitSceneSettingsUpdateResponseForCompletion:afterSceneUpdateWork:]
26 UIKitCore                      0x20ef28 -[UIScene scene:didUpdateWithDiff:transitionContext:completion:]
27 UIKitCore                      0x20e47c -[UIApplication workspace:didCreateScene:withTransitionContext:completion:]
28 UIKitCore                      0x20e208 -[UIApplicationSceneClientAgent scene:didInitializeWithEvent:completion:]
29 FrontBoardServices             0x3500 -[FBSScene _callOutQueue_agent_didCreateWithTransitionContext:completion:]
30 FrontBoardServices             0x4251c __92-[FBSWorkspaceScenesClient createSceneWithIdentity:parameters:transitionContext:completion:]_block_invoke.78
31 FrontBoardServices             0x7294 -[FBSWorkspace _calloutQueue_executeCalloutFromSource:withBlock:]
32 FrontBoardServices             0x42154 __92-[FBSWorkspaceScenesClient createSceneWithIdentity:parameters:transitionContext:completion:]_block_invoke
33 libdispatch.dylib              0x3fdc _dispatch_client_callout
34 libdispatch.dylib              0x7a5c _dispatch_block_invoke_direct
35 FrontBoardServices             0x113b0 __FBSSERIALQUEUE_IS_CALLING_OUT_TO_A_BLOCK__
36 FrontBoardServices             0x10f4c -[FBSSerialQueue _targetQueue_performNextIfPossible]
37 FrontBoardServices             0x1372c -[FBSSerialQueue _performNextFromRunLoopSource]
38 CoreFoundation                 0xd5f54 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
39 CoreFoundation                 0xe232c __CFRunLoopDoSource0
40 CoreFoundation                 0x66270 __CFRunLoopDoSources0
41 CoreFoundation                 0x7bba8 __CFRunLoopRun
42 CoreFoundation                 0x80ed4 CFRunLoopRunSpecific
43 GraphicsServices               0x1368 GSEventRunModal
44 UIKitCore                      0x3a23d0 -[UIApplication _run]
45 UIKitCore                      0x3a2034 UIApplicationMain
46 App Name                       0x429e38 main + 26 (AppDelegate.swift:26)
47 ???                            0x1da9c8960 (Missing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working RollbarCommon RollbarCommon module specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rollbar-Apple causing crashes in application
3 participants