Skip to content

feat: SG-35309: Set and save OCIO environment variable if missing and config is chosen#1263

Open
deltag0 wants to merge 11 commits into
AcademySoftwareFoundation:mainfrom
deltag0:OCIO-environment-variable-output-fix
Open

feat: SG-35309: Set and save OCIO environment variable if missing and config is chosen#1263
deltag0 wants to merge 11 commits into
AcademySoftwareFoundation:mainfrom
deltag0:OCIO-environment-variable-output-fix

Conversation

@deltag0
Copy link
Copy Markdown

@deltag0 deltag0 commented May 7, 2026

feat: Set and save OCIO environment variable if missing and config is chosen

Summarize your change.

When a user loads a valid OCIO config after opening a file and sets the OCIO as an active display with the environment variable OCIO not set, there used to be an error log. The error log is now gone and the environment variable is set after selecting a config.
Moreover, the config path is saved to user preferences after choosing it.

Describe the reason for the change.

Redundant error logs and console pop ups because once the user loads the OCIO config with no environment variable there's already an error log.

Describe what you have tested and on which operating system.

Tested on Mac CY2025

Add a list of changes, and note any that might need special attention during the review.

N/A

If possible, provide screenshots.

snipit_2026-05-07T11-15-47 251281

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 7, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

deltag0 added 2 commits May 7, 2026 11:31
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
@deltag0 deltag0 force-pushed the OCIO-environment-variable-output-fix branch from 394d400 to 77ffaa4 Compare May 7, 2026 15:32
@deltag0 deltag0 marked this pull request as ready for review May 7, 2026 15:33
@TommySny TommySny added the PR: In Progress PR is being reviewed by code reviewer. label May 11, 2026
deltag0 added 4 commits May 12, 2026 13:35
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Comment thread src/plugins/rv-packages/ocio_source_setup/ocio_source_setup.py Outdated
@deltag0 deltag0 changed the title fix: SG-35309: Remove OCIO output when set to active with env variable unset fix: SG-35309: Set and save OCIO environment variable if missing and config is chosen May 13, 2026
@deltag0 deltag0 changed the title fix: SG-35309: Set and save OCIO environment variable if missing and config is chosen feat: SG-35309: Set and save OCIO environment variable if missing and config is chosen May 13, 2026
deltag0 added 3 commits May 13, 2026 12:08
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Signed-off-by: deltag0 <victor.terme@autodesk.com>
Comment thread src/plugins/rv-packages/ocio_source_setup/ocio_source_setup.py
Copy link
Copy Markdown
Contributor

@bernie-laberge bernie-laberge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: deltag0 <victor.terme@autodesk.com>
Comment thread src/plugins/rv-packages/ocio_source_setup/ocio_source_setup.py Outdated
Signed-off-by: deltag0 <victor.terme@autodesk.com>
@markreidvfx
Copy link
Copy Markdown
Contributor

Setting environmental variables at runtime while other thread are running and is not safe. If any thread just so happens to be reading one at the same time as another thread is setting a realloc can happen and getenv will read after free and potential crash. Linux at least performs not shared locking between getenv and setenv as far as I'm aware. I recently fix a bug with some of our RV plugins related exactly to this.

@bernie-laberge
Copy link
Copy Markdown
Contributor

Setting environmental variables at runtime while other thread are running and is not safe. If any thread just so happens to be reading one at the same time as another thread is setting a realloc can happen and getenv will read after free and potential crash. Linux at least performs not shared locking between getenv and setenv as far as I'm aware. I recently fix a bug with some of our RV plugins related exactly to this.

According to our robot friend, @markreidvfx is right:
Mark is right, and this matters for the PR

The claim is technically correct, well-documented, and has bitten real applications repeatedly:

  • environ is a flat char** array maintained by libc. setenv/putenv may realloc it and replace string pointers.
  • glibc's setenv takes an internal lock, but getenv does not acquire it — see sourceware #15607 (https://sourceware.org/bugzilla/show_bug.cgi?id=15607). POSIX explicitly only guarantees getenv thread-safety
    in the absence of concurrent modifications.
  • The crash mode is exactly what Mark described: a getenv caller dereferences a char* that setenv just freed.
  • This isn't theoretical — Rust shipped a CVE around it (CVE-2024-43807 / set_var (https://blog.rust-lang.org/2024/09/19/Rust-1.82-pre-announce-set_var-unsafe.html) marking set_var unsafe in 1.82), Go has the
    same hazard, OpenSSL/curl have warnings about it. It's a known footgun, not an edge case.

I think we should explore an alternative way of setting an OCIO config without having to set the OCIO environment variable.
Thank you @markreidvfx !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: In Progress PR is being reviewed by code reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants