Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

DeepMind logger holds const char* fields that it uses for some of its naming and indicators #9877

Merged
merged 2 commits into from
Jan 6, 2021

Conversation

TimothyBanks
Copy link
Contributor

Change Description

DeepMind logger holds const char* fields that it uses for some of its naming and indicators. This is an issue as the logger is a separate thread and these pointers can go stale. It is working currently as a side effect that C style string literals will be in the code section of the program and therefore globally available with a lifetime of the entire program. If you were to pass it a pointer from an std::string or std::string_view this can lead to dangling pointers and corruption.

Change Type

Select ONE:

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Testing Changes

Select ANY that apply:

  • New Tests
  • Existing Tests
  • Test Framework
  • CI System
  • Other

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@heifner
Copy link
Contributor

heifner commented Jan 5, 2021

dmlog_appender does not run a separate thread. It might be called from multiple different threads in the future, but currently only called from the main thread.

@TimothyBanks TimothyBanks merged commit 111378f into develop Jan 6, 2021
@TimothyBanks TimothyBanks deleted the deepmind branch January 6, 2021 00:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants