Skip to content

Fix multiple bugs found during code review#83

Merged
jgowdy-godaddy merged 1 commit intomainfrom
fix-bugs
Mar 3, 2026
Merged

Fix multiple bugs found during code review#83
jgowdy-godaddy merged 1 commit intomainfrom
fix-bugs

Conversation

@jgowdy-godaddy
Copy link
Contributor

Summary

  • Fix Setup() race condition: Replace Load+Store with CompareAndSwap to prevent concurrent initialization overwriting globalSessionFactory. Roll back initialized flag on factory creation failure.
  • Fix wrong variable in error log: SetupJson line 84 logged result (from BufferToJsonStruct) instead of stringResult (from BufferToString)
  • Fix panic re-throw in FFI exports: All exported functions were catching panics then re-panicking, which is undefined behavior across an FFI boundary. Now returns ERR_PANIC (-106) via named return values instead.
  • Make DisableZeroCopy atomic: Was a plain bool written in SetupJson and read in encryptData — data race under -race. Now uses atomic.Bool.
  • Close DB connection in Shutdown: dbconnection was opened but never closed, leaking the resource.
  • Add missing log in Decrypt not-initialized: Encrypt logged "not initialized" but Decrypt returned silently. Now consistent.
  • Check os.Setenv errors in SetEnv: Previously silently ignored failures. Now logs and returns ERR_BAD_CONFIG.

Test plan

  • All 32 tests pass
  • go vet clean
  • go build clean

- Fix Setup() race condition: use CompareAndSwap instead of Load/Store
- Fix wrong variable in SetupJson error log (result vs stringResult)
- Fix panic re-throw in FFI exports: return ERR_PANIC instead of
  re-panicking across FFI boundary (undefined behavior)
- Make DisableZeroCopy atomic to prevent data race
- Close DB connection in Shutdown to prevent resource leak
- Add missing error log in Decrypt not-initialized path
- Check os.Setenv errors in SetEnv instead of silently ignoring
@jgowdy-godaddy jgowdy-godaddy merged commit 9d0dbbe into main Mar 3, 2026
4 checks passed
@jgowdy-godaddy jgowdy-godaddy deleted the fix-bugs branch March 3, 2026 23:10
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.

2 participants