dmsurti / AssimpKit

A library (macOS, iOS) that converts the files supported by Assimp to Scene Kit scenes.
http://assimpkit.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
477 stars 54 forks source link

Crash when loading unsupported file #79

Closed AlicanC closed 7 years ago

AlicanC commented 7 years ago

On the following line https://github.com/dmsurti/AssimpKit/blob/8d5997c4f52ab567a978e8b0a4916b49a5694535/AssimpKit/Code/Model/AssimpImporter.m#L131 even if aiScene is not NULL, aiScene->mRootNode can be, and makes [AssimpImporter makeSCNNodeFromAssimpNode:inScene:atPath:] crash.

I am not sure if aiGetErrorString() has anything when this happens, but if it does, printing it can also be useful.

dmsurti commented 7 years ago

Please submit the error trace and the model file which causes the problem. Thanks.

AlicanC commented 7 years ago

This is the model: https://sketchfab.com/models/d6c802a74a174c8c805deb20186d1877 (I accidentally tried to load the GLTF version with AssimpKit.)

(lldb)  po [NSThread callStackSymbols] 
AssimpKit was compiled with optimization - stepping may behave oddly; variables may not be available.
<_NSCallStackArray 0x1c1249840>(
0   ???                                 0x00000001101e88a4 0x0 + 4565403812,
1   *REDACTED*
2   Foundation                          0x00000001837c1b64 <redacted> + 100,
3   AssimpKit                           0x00000001079286ec -[AssimpImporter makeSCNNodeFromAssimpNode:inScene:atPath:] + 116,
4   AssimpKit                           0x00000001079285a0 -[AssimpImporter makeSCNSceneFromAssimpScene:atPath:] + 120,
5   AssimpKit                           0x0000000107928480 -[AssimpImporter importScene:postProcessFlags:] + 96,
6   *REDACTED*
7   *REDACTED*
8   *REDACTED*
9   *REDACTED*
10  *REDACTED*
11  Result                              0x0000000107512c3c _T06ResultAAOAByxq_GxyKc7attempt_tcfC + 200,
12  *REDACTED*
13  *REDACTED*
14  BrightFutures                       0x00000001063beda8 _T013BrightFutures9AsyncTypePA2aBRz6Result0E8Protocol5ValueRpzlE7flatMapAA6FutureCyqd__AF_5ErrorQZGyyycc_A2DOyqd__ALGAF_AFQZc1ftlFAmPcfU_ + 416,
15  BrightFutures                       0x00000001063be85c _T05Value_AAQZ13BrightFutures6FutureCyqd__AA_5ErrorQZGIxio_AbHIxir_AC9AsyncTypeRz6Result0H8ProtocolAARpzr__lTR + 36,
16  BrightFutures                       0x00000001063bf430 _T013BrightFutures9AsyncTypePA2aBRz6Result0E8Protocol5ValueRpzlE3mapAA6FutureCyqd__AF_5ErrorQZGyyycc_qd__AF_AFQZc1ftlFyAGcfU_yANcfU_ + 320,
17  BrightFutures                       0x00000001063bf4e8 _T05Value_AAQZIxi_ABytIxir_13BrightFutures9AsyncTypeRz6Result0F8ProtocolAARpzr__lTR + 32,
18  Result                              0x000000010751375c _T06ResultAAO8analysisqd__qd__xc9ifSuccess_qd__q_c0C7FailuretlF + 516,
19  Result                              0x0000000107514d54 _T06ResultAAOyxq_GAA0A8ProtocolAAs5ErrorR_r0_lAaDP8analysisqd__qd__5ValueQzc9ifSuccess_qd__AEQzc0F7FailuretlFTW + 296,
20  BrightFutures                       0x00000001063bf2b8 _T013BrightFutures9AsyncTypePA2aBRz6Result0E8Protocol5ValueRpzlE3mapAA6FutureCyqd__AF_5ErrorQZGyyycc_qd__AF_AFQZc1ftlFyAGcfU_ + 584,
21  BrightFutures                       0x00000001063b9b98 _T013BrightFutures5AsyncC10onCompleteACyxGXDyyycc_yxc8callbacktFyxcfU_yycfU_yycfU_ + 160,
22  BrightFutures                       0x00000001063c8018 _T0So17DispatchSemaphoreC13BrightFuturesE7contextyyyccfgyyyccfU_ + 84,
23  BrightFutures                       0x00000001063b9ac4 _T013BrightFutures5AsyncC10onCompleteACyxGXDyyycc_yxc8callbacktFyxcfU_yycfU_ + 328,
24  BrightFutures                       0x00000001063b81d0 _T0Ix_IyB_TR + 48,
25  libdispatch.dylib                   0x00000001095c149c _dispatch_call_block_and_release + 24,
26  libdispatch.dylib                   0x00000001095c145c _dispatch_client_callout + 16,
27  libdispatch.dylib                   0x00000001095cd56c _dispatch_queue_override_invoke + 980,
28  libdispatch.dylib                   0x00000001095d2b54 _dispatch_root_queue_drain + 616,
29  libdispatch.dylib                   0x00000001095d2880 _dispatch_worker_thread3 + 136,
30  libsystem_pthread.dylib             0x0000000182b0b130 _pthread_wqthread + 1268,
31  libsystem_pthread.dylib             0x0000000182b0ac30 start_wqthread + 4
)
dmsurti commented 7 years ago

I checked and the FBX file is imported fine both in iOS and macOS sample apps. Please ensure that you have installed AssimpKit correctly and if you are using it in your app, please refer to the instructions/tutorial.

For installation, you can also refer to: #77.

Sorry, but gltf is not supported right now; see #72.

AlicanC commented 7 years ago

Crashing should not be considered a correct behaviour in any case.

One could make an app in which users can select their own model files. In that case, a user can select an unsupported file (just like I did) and the app would crash and apps shouldn't crash.

AssimpKit should error instead of crashing so app developers like me can display helpful messages to their users.

dmsurti commented 7 years ago

@AlicanC Please read the manual before offering advice as to how things should be done.

There is a specific method https://dmsurti.github.io/AssimpKit/appledocs/html/Categories/SCNScene+AssimpImport.html#//api/name/canImportFileExtension: which you should call to check if the format you are trying to load is supported or not!

alexanderwallin commented 6 years ago

I've had EXC_BAD_ACCESS crashes for .obj files that theoretically are supported. To learn whatever was wrong with the model would be ace!

alexanderwallin commented 6 years ago

Okay, just found and used the AssimpKit_Process_ValidateDataStructure. Would it be a good idea to always have this enabled?