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

Eschew global static object, to avoid issue with double-free #241

Merged
merged 2 commits into from
Dec 29, 2020

Conversation

hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Dec 29, 2020

When I was working on rapidsai/cuml#3316, I noticed that both libraries libtreelite.so and libtreelite_runtime.so contain a reference to the global static constant typeinfo_table. Since the constant was of type std::unordered_map<std::string, std::string>, it contains a non-trivial destructor. When both libraries are unloaded, both will attempt to simultaneously free the constant typeinfo_table from memory, leading to double-free and thus undefined behavior.

Fix: Eliminate the use of global static constant and replace it with a function instead. Allocating memory in the local scope is safer and does not risk the possibility of double-free.

@hcho3 hcho3 mentioned this pull request Dec 29, 2020
8 tasks
@hcho3 hcho3 merged commit daf595d into dmlc:mainline Dec 29, 2020
@hcho3 hcho3 deleted the avoid_global_static_constant branch December 29, 2020 18:33
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request Jan 7, 2021
Depends on dmlc/treelite#235, dmlc/treelite#236, dmlc/treelite#237, dmlc/treelite#240, dmlc/treelite#241, dmlc/treelite#242, dmlc/treelite#243

Depends on rapidsai/integration#201

Closes #2160
Closes #2795
Closes #2817

TODOs
- [x] Get code review for dmlc/treelite#235 and dmlc/treelite#236 and merge them
- [x]  Get code review for dmlc/treelite#237 and merge them
- [x] Merge dmlc/treelite#240, dmlc/treelite#241, dmlc/treelite#242, dmlc/treelite#243
- [x] Run tests locally
- [x] Tag a new version of Treelite
- [x] Release a new Conda package with the new version tag
- [x] Update `meta.yml` and the integration package to use the new Treelite
- [x] Run tests (this time build with new Conda package)

Authors:
  - Hyunsu Cho <chohyu01@cs.washington.edu>

Approvers:
  - AJ Schmidt (@ajschmidt8)
  - William Hicks (@wphicks)
  - John Zedlewski (@JohnZed)

URL: #3316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant