ed/Odin
1
0
Fork 0
mirror of https://github.com/Ed94/Odin.git synced 2026-08-03 05:08:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4907 from Feoramund/os2-fix-env-linux

Browse Source 
Fix data races in `os2/env_linux.odin`
This commit is contained in:
gingerBill
2025-03-05 13:03:07 +00:00
committed by GitHub
parent 69b6c59ea6 2ab1ca29e6
commit 951bef4ade
2 changed files with 22 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/os/os2/env_linux.odin
+21 -29
View File
@@ -20,19 +20,18 @@ NOT_FOUND :: -1
// the environment is a 0 delimited list of <key>=<value> strings // the environment is a 0 delimited list of <key>=<value> strings
_env: [dynamic]string _env: [dynamic]string
_env_mutex: sync.Mutex _env_mutex: sync.Recursive_Mutex
// We need to be able to figure out if the environment variable // We need to be able to figure out if the environment variable
// is contained in the original environment or not. This also // is contained in the original environment or not. This also
// serves as a flag to determine if we have built _env. // serves as a flag to determine if we have built _env.
_org_env_begin: uintptr _org_env_begin: uintptr // atomic
_org_env_end: uintptr _org_env_end: uintptr // guarded by _env_mutex
// Returns value + index location into _env // Returns value + index location into _env
// or -1 if not found // or -1 if not found
_lookup :: proc(key: string) -> (value: string, idx: int) { _lookup :: proc(key: string) -> (value: string, idx: int) {
sync.mutex_lock(&_env_mutex) sync.guard(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex)
for entry, i in _env { for entry, i in _env {
if k, v := _kv_from_entry(entry); k == key { if k, v := _kv_from_entry(entry); k == key {
@@ -43,7 +42,7 @@ _lookup :: proc(key: string) -> (value: string, idx: int) {
} }
_lookup_env :: proc(key: string, allocator: runtime.Allocator) -> (value: string, found: bool) { _lookup_env :: proc(key: string, allocator: runtime.Allocator) -> (value: string, found: bool) {
if _org_env_begin == 0 { if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) == 0 {
_build_env() _build_env()
} }
@@ -55,9 +54,10 @@ _lookup_env :: proc(key: string, allocator: runtime.Allocator) -> (value: string
} }
_set_env :: proc(key, v_new: string) -> Error { _set_env :: proc(key, v_new: string) -> Error {
if _org_env_begin == 0 { if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) == 0 {
_build_env() _build_env()
} }
sync.guard(&_env_mutex)
// all key values are stored as "key=value\x00" // all key values are stored as "key=value\x00"
kv_size := len(key) + len(v_new) + 2 kv_size := len(key) + len(v_new) + 2
@@ -65,8 +65,6 @@ _set_env :: proc(key, v_new: string) -> Error {
if v_curr == v_new { if v_curr == v_new {
return nil return nil
} }
sync.mutex_lock(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex)
unordered_remove(&_env, idx) unordered_remove(&_env, idx)
@@ -101,16 +99,15 @@ _set_env :: proc(key, v_new: string) -> Error {
intrinsics.mem_copy_non_overlapping(&val_slice[0], raw_data(v_new), len(v_new)) intrinsics.mem_copy_non_overlapping(&val_slice[0], raw_data(v_new), len(v_new))
val_slice[len(v_new)] = 0 val_slice[len(v_new)] = 0
sync.mutex_lock(&_env_mutex)
append(&_env, string(k_addr[:kv_size - 1])) append(&_env, string(k_addr[:kv_size - 1]))
sync.mutex_unlock(&_env_mutex)
return nil return nil
} }
_unset_env :: proc(key: string) -> bool { _unset_env :: proc(key: string) -> bool {
if _org_env_begin == 0 { if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) == 0 {
_build_env() _build_env()
} }
sync.guard(&_env_mutex)
v: string v: string
i: int i: int
@@ -118,15 +115,13 @@ _unset_env :: proc(key: string) -> bool {
return false return false
} }
sync.mutex_lock(&_env_mutex)
unordered_remove(&_env, i) unordered_remove(&_env, i)
sync.mutex_unlock(&_env_mutex)
if _is_in_org_env(v) { if _is_in_org_env(v) {
return true return true
} }
// if we got this far, the envrionment variable // if we got this far, the environment variable
// existed AND was allocated by us. // existed AND was allocated by us.
k_addr, _ := _kv_addr_from_val(v, key) k_addr, _ := _kv_addr_from_val(v, key)
runtime.heap_free(k_addr) runtime.heap_free(k_addr)
@@ -134,8 +129,7 @@ _unset_env :: proc(key: string) -> bool {
} }
_clear_env :: proc() { _clear_env :: proc() {
sync.mutex_lock(&_env_mutex) sync.guard(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex)
for kv in _env { for kv in _env {
if !_is_in_org_env(kv) { if !_is_in_org_env(kv) {
@@ -145,14 +139,16 @@ _clear_env :: proc() {
clear(&_env) clear(&_env)
// nothing resides in the original environment either // nothing resides in the original environment either
_org_env_begin = ~uintptr(0) intrinsics.atomic_store_explicit(&_org_env_begin, ~uintptr(0), .Release)
_org_env_end = ~uintptr(0) _org_env_end = ~uintptr(0)
} }
_environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error) { _environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error) {
if _org_env_begin == 0 { if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) == 0 {
_build_env() _build_env()
} }
sync.guard(&_env_mutex)
env := make([dynamic]string, 0, len(_env), allocator) or_return env := make([dynamic]string, 0, len(_env), allocator) or_return
defer if err != nil { defer if err != nil {
for e in env { for e in env {
@@ -161,8 +157,6 @@ _environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error
delete(env) delete(env)
} }
sync.mutex_lock(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex)
for entry in _env { for entry in _env {
s := clone_string(entry, allocator) or_return s := clone_string(entry, allocator) or_return
append(&env, s) append(&env, s)
@@ -174,7 +168,7 @@ _environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error
// The entire environment is stored as 0 terminated strings, // The entire environment is stored as 0 terminated strings,
// so there is no need to clone/free individual variables // so there is no need to clone/free individual variables
export_cstring_environment :: proc(allocator: runtime.Allocator) -> []cstring { export_cstring_environment :: proc(allocator: runtime.Allocator) -> []cstring {
if _org_env_begin == 0 { if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) == 0 {
// The environment has not been modified, so we can just // The environment has not been modified, so we can just
// send the original environment // send the original environment
org_env := _get_original_env() org_env := _get_original_env()
@@ -182,12 +176,11 @@ export_cstring_environment :: proc(allocator: runtime.Allocator) -> []cstring {
for ; org_env[n] != nil; n += 1 {} for ; org_env[n] != nil; n += 1 {}
return slice.clone(org_env[:n + 1], allocator) return slice.clone(org_env[:n + 1], allocator)
} }
sync.guard(&_env_mutex)
// NOTE: already terminated by nil pointer via + 1 // NOTE: already terminated by nil pointer via + 1
env := make([]cstring, len(_env) + 1, allocator) env := make([]cstring, len(_env) + 1, allocator)
sync.mutex_lock(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex)
for entry, i in _env { for entry, i in _env {
env[i] = cstring(raw_data(entry)) env[i] = cstring(raw_data(entry))
} }
@@ -195,15 +188,14 @@ export_cstring_environment :: proc(allocator: runtime.Allocator) -> []cstring {
} }
_build_env :: proc() { _build_env :: proc() {
sync.mutex_lock(&_env_mutex) sync.guard(&_env_mutex)
defer sync.mutex_unlock(&_env_mutex) if intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) != 0 {
if _org_env_begin != 0 {
return return
} }
_env = make(type_of(_env), runtime.heap_allocator()) _env = make(type_of(_env), runtime.heap_allocator())
cstring_env := _get_original_env() cstring_env := _get_original_env()
_org_env_begin = uintptr(rawptr(cstring_env[0])) intrinsics.atomic_store_explicit(&_org_env_begin, uintptr(rawptr(cstring_env[0])), .Release)
for i := 0; cstring_env[i] != nil; i += 1 { for i := 0; cstring_env[i] != nil; i += 1 {
bytes := ([^]u8)(cstring_env[i]) bytes := ([^]u8)(cstring_env[i])
n := len(cstring_env[i]) n := len(cstring_env[i])
@@ -235,5 +227,5 @@ _kv_addr_from_val :: #force_inline proc(val: string, key: string) -> ([^]u8, [^]
_is_in_org_env :: #force_inline proc(env_data: string) -> bool { _is_in_org_env :: #force_inline proc(env_data: string) -> bool {
addr := uintptr(raw_data(env_data)) addr := uintptr(raw_data(env_data))
return addr >= _org_env_begin && addr < _org_env_end return addr >= intrinsics.atomic_load_explicit(&_org_env_begin, .Acquire) && addr < _org_env_end
} }
core/os/os2/env_posix.odin
+1 -1
View File
@@ -57,7 +57,7 @@ _clear_env :: proc() {
} }
_environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error) { _environ :: proc(allocator: runtime.Allocator) -> (environ: []string, err: Error) {
n := 0 n := 0
for entry := posix.environ[0]; entry != nil; n, entry = n+1, posix.environ[n] {} for entry := posix.environ[0]; entry != nil; n, entry = n+1, posix.environ[n] {}
r := make([dynamic]string, 0, n, allocator) or_return r := make([dynamic]string, 0, n, allocator) or_return