vr: Handle exceptional scenarios in jsoncpp/openvr third_party code without crashing |
||||||||||||
Issue descriptionThere are some exceptions being thrown in jsoncpp in third_party\openvr. We should avoid exceptions and avoid crashing. From Bruce: The solution we have used elsewhere is longjmp - the old-style way of throwing exceptions - but switching to that is not a change to be done lightly.
,
Dec 7 2017
Bill. Is this fixed by crrev.com/c/801094 or is there anything else left to do? In other word, can we close this bug?
,
Dec 7 2017
Since Chromium doesn't build with support for exceptions these 'throw' statements are *not* an effective way to handle failures. I'm not sure what they will do but process termination seems likely.
,
Dec 7 2017
We need to keep this, and its something we need to address before shipping OpenVR support. For testing, exceptions are unlikely to occur and we can (for the short term) ignore them.
,
Dec 7 2017
,
Jul 4
,
Jul 12
,
Jul 12
,
Jul 12
,
Aug 7
Removing Internals>VR component and assigning to Internals>XR
,
Aug 7
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f969bff5532233afff5564f947b453b5dac6be6 commit 2f969bff5532233afff5564f947b453b5dac6be6 Author: Bill Orr <billorr@chromium.org> Date: Tue Oct 23 18:33:04 2018 Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/heads/master@{#602033} [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/device/vr/BUILD.gn [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/third_party/openvr/BUILD.gn [modify] https://crrev.com/2f969bff5532233afff5564f947b453b5dac6be6/third_party/openvr/README.chromium [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/headers/openvr_api.cs [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/headers/openvr_api.json [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/json/json-forwards.h [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/json/json.h [delete] https://crrev.com/af38308b7112bdfbc0cfcc27f966314accc471d0/third_party/openvr/src/src/jsoncpp.cpp
,
Oct 23
We use the jsoncpp from third_party, which doesn't throw.
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5651b7e8d2688f718831d5282f910b70ab0f9b9b Commit: 5651b7e8d2688f718831d5282f910b70ab0f9b9b Author: billorr@chromium.org Commiter: billorr@chromium.org Date: 2018-10-26 20:27:38 +0000 UTC Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602033}(cherry picked from commit 2f969bff5532233afff5564f947b453b5dac6be6) Reviewed-on: https://chromium-review.googlesource.com/c/1302859 Reviewed-by: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#351} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5651b7e8d2688f718831d5282f910b70ab0f9b9b commit 5651b7e8d2688f718831d5282f910b70ab0f9b9b Author: Bill Orr <billorr@chromium.org> Date: Fri Oct 26 20:27:38 2018 Remove duplicate jsoncpp from OpenVR OpenVR by default includes its own jsoncpp. However, we have one in third_party already, so we don't need OpenVR's copy. BUG= 780987 , 790814 , 896087 Change-Id: Iac57998856ac72b36b6b731b3860ca67ae7c3f45 Reviewed-on: https://chromium-review.googlesource.com/c/1294669 Reviewed-by: Klaus Weidner <klausw@chromium.org> Commit-Queue: Bill Orr <billorr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602033}(cherry picked from commit 2f969bff5532233afff5564f947b453b5dac6be6) Reviewed-on: https://chromium-review.googlesource.com/c/1302859 Reviewed-by: Bill Orr <billorr@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#351} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/device/vr/BUILD.gn [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/third_party/openvr/BUILD.gn [modify] https://crrev.com/5651b7e8d2688f718831d5282f910b70ab0f9b9b/third_party/openvr/README.chromium [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/headers/openvr_api.cs [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/headers/openvr_api.json [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/json/json-forwards.h [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/json/json.h [delete] https://crrev.com/fd47f329fe335046fcf28660866a8ca4ff2328dc/third_party/openvr/src/src/jsoncpp.cpp |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by brucedaw...@chromium.org
, Dec 1 2017The 'throw' statements had to be removed because of this compiler error: cannot use 'throw' with exceptions disabled which is reasonable enough - we disable C++ exceptions when building Chrome. Both longjmp and structured exception handling have the problem that intervening destructors are not run, leading to potential resource leaks. Also, structured exception handling is not portable. See crrev.com/c/801094, especially the try-job failures on patch-set 2, for more details.