microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.8k stars 6.3k forks source link

[azure-kinect-sensor-sdk] Run-time crash due to multiple libraries trying to use the same log-file. #34871

Open MartinEekGerhardsen opened 10 months ago

MartinEekGerhardsen commented 10 months ago

Describe the bug When using the k4arecord library, both k4a and k4arecord seem to try to log to the same logfile.

Environment

To Reproduce Steps to reproduce the behavior:

  1. Create new folder with these files: CMakeLists.txt
    add_executable (main "main.cpp" )
    find_package(k4a CONFIG REQUIRED)
    find_package(k4arecord CONFIG REQUIRED)
    target_link_libraries(main PRIVATE k4a::k4a k4a::k4arecord)

    vcpkg.json

    { "dependencies": [ "azure-kinect-sensor-sdk" ] }
#include <iostream>
#include <cstdlib>

#include <k4arecord/playback.hpp>

int main()
{
    std::cout << "K4A_ENABLE_LOG_TO_A_FILE " << std::getenv("K4A_ENABLE_LOG_TO_A_FILE") << '\n';
    std::cout << "K4A_RECORD_ENABLE_LOG_TO_A_FILE " << std::getenv("K4A_RECORD_ENABLE_LOG_TO_A_FILE") << '\n';
    k4a::playback pb = k4a::playback::open("C:\\dev\\testdata\\test.mkv");

    return 0;
}
  1. Set the following env variables:
    $env:K4A_ENABLE_LOG_TO_A_FILE = "0"
    $env:K4A_RECORD_ENABLE_LOG_TO_A_FILE = "0"

    or

    $env:K4A_ENABLE_LOG_TO_A_FILE = "k4abase.log"
    $env:K4A_RECORD_ENABLE_LOG_TO_A_FILE = "k4arecord.log"
  2. Set up cmake
    cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=C:/dev/vcpkg/scripts/buildsystems/vcpkg.cmake
    cmake --build .\build\ --config Debug
    .\build\Debug\test.exe

Expected behavior The program shows the current env varables (to ensure that I know these are set) and then the program executes without issue.

Failure logs If I set the env variables to "0", I get:

ERROR: Logger unable to open log file "k4a.log".

If I set the env variables to the two separate log files, I get:

ERROR: Logger unable to open log file "k4abase.log"

Furthermore, I get this debug error: bilde

Additional context I believe this is an issue with vcpkg, as I have done this versions of the azure kinect sensor sdk installed to this system without vcpkg. When I use these, I don't get the same error. However, it would be much simpler for me to use vcpkg as I also need the pcl library, which is extremely difficult to install without vcpkg.

FrankXie05 commented 10 months ago

@MartinEekGerhardsen Thanks for posting this issue. Could you please use the full path of this file?

$env:K4A_ENABLE_LOG_TO_A_FILE = "k4abase.log"
$env:K4A_RECORD_ENABLE_LOG_TO_A_FILE = "k4arecord.log"
MartinEekGerhardsen commented 10 months ago

@FrankXie05 Thank you for the reply! I am not sure I understood what you wanted me to do but I tried setting the full path and got this result:

bilde

As you can see, the same error is produced, and both log files are created in the directory I'm executing the code from.

MartinEekGerhardsen commented 10 months ago

@FrankXie05 Is there any other information I can provide?

FrankXie05 commented 10 months ago

Could you please provide your test.mkv?(Make sure it conforms to the format of Azure Kinect) :)

MartinEekGerhardsen commented 10 months ago

I have uploaded it here: https://fastupload.io/F3VfZPxSfjReXGQ/file The file should be fine, as I recorded it with the Azure Kinect record command.

MartinEekGerhardsen commented 9 months ago

Hello @FrankXie05, have you had any time to look at this problem? I haven't been able to figure anything more out on my end 😅

MartinEekGerhardsen commented 9 months ago

ping @FrankXie05

FrankXie05 commented 9 months ago

@MartinEekGerhardsen Sorry for the late reply. It looks like this is a functional problem of azure-kinect-sensor-sdk, not a problem of vcpkg. I'm not an expert on azure-kinect-sensor-sdk, you can submit an issue to https://github.com/microsoft/Azure-Kinect-Sensor-SDK, I think you can find the answer there. :)

MartinEekGerhardsen commented 9 months ago

@FrankXie05 Thanks for the help! I'll do that, and reference this issue. I'll also keep this issue open for now. The reason why I thought this issue could be related to vcpkg is because I was able to get the code to run when I used system installs instead of vcpkg, but I'd obviously prefer having everything defined using vcpkg. Here is a link to the new issue. If I figure something out, I'll update here.

thundercarrot commented 7 months ago

@MartinEekGerhardsen I'm seeing this too and I think the crash is caused by executing spdlog::stdout_logger_mt(K4A_LOGGER) more than once. See logging.cpp:209.

I believe the error you are seeing when setting those other env variables to two different file names is due to the fact that the logging setup functions are called THREE times, twice for the base k4a.lib and once for k4arecord.lib. This is at least handled without crashing however (hence the error message you see).

A work around is to set K4A_ENABLE_LOG_TO_STDOUT to "0".

How does k4aviewer avoid this I wonder?

MartinEekGerhardsen commented 7 months ago

Yeah that's what I ended up concluding as well. I have no idea why the k4arecord library calls tries to use the same logger, as from the docs it seemed like it should have its own separate logger. I think the primary problem is that two different libraries tries to access the same file, so the two cases in k4a.lib is fine, though when adding k4arecord.lib, the error is triggered.

No idea how k4aviewer avoids this though... Could be interesting to investigate at some point.

Still, as it seems like azure kinect has been discontinued, I'm not sure any updates will ever come from this...

thundercarrot commented 7 months ago

The problem doesn't appear when running with k4aviewer compiled with the sdk not under vcpkg.

I think the issue is that when compiled with vcpkg, azure-kinect-sensor-sdk uses the version of spdlog that vcpkg pulls down. That seems to be newer than the one used by the vanilla k4a repo, and evidently includes a breaking change.

MartinEekGerhardsen commented 7 months ago

Yeah that was why I initially posted this issue here, as the same code example worked with my custom version of Azure-Kintect-Sensor-SDK.

I notice that Azure-Kintect-Sensor-SDK links to spdlog 0.17.0. Now I have no idea if there are any critical vulnerabilities in the code since 0.17.0 (spdlog is at 1.13.0 atm), but if this works, perhaps we can just depend on spdlog version 0.17.0? `

github-actions[bot] commented 1 month ago

This is an automated message. Per our repo policy, stale issues get closed if there has been no activity in the past 180 days. The issue will be automatically closed in 14 days. If you wish to keep this issue open, please add a new comment.

MartinEekGerhardsen commented 1 month ago

@FrankXie05 any updates on this?

FrankXie05 commented 1 month ago

It looks like the upstream is not responding.🧐