fix: Critical code review fixes — race conditions, panic recovery, LTRIM/RTRIM
CRITICAL fixes: - fileio.go: Add sync.Mutex to file handle table (race condition #2) allocHandle/getHandle/removeHandle thread-safe helpers - goroutine.go: Add defer/recover to GoLaunch/GoLaunchBlock Goroutine panic no longer crashes entire process (#5) HIGH fixes: - strings.go: Implement proper LTrim (TrimLeft) and RTrim (TrimRight) Previously both aliased to AllTrim — silent semantic bug (#18) - register.go: TRIM = RTrim (Harbour compatible) From 53-issue senior code review. Remaining: 47 issues (HIGH: 10, MEDIUM: 18, LOW: 16, CRITICAL: 3) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"version": "2.0",
|
"version": "2.0",
|
||||||
"lastUpdated": "2026-03-31T02:56:59.267Z",
|
"lastUpdated": "2026-04-01T01:16:33.700Z",
|
||||||
"activeFeatures": [
|
"activeFeatures": [
|
||||||
"hbrt",
|
"hbrt",
|
||||||
"hbrtl",
|
"hbrtl",
|
||||||
@@ -33,9 +33,9 @@
|
|||||||
"documents": {},
|
"documents": {},
|
||||||
"timestamps": {
|
"timestamps": {
|
||||||
"started": "2026-03-27T09:33:04.512Z",
|
"started": "2026-03-27T09:33:04.512Z",
|
||||||
"lastUpdated": "2026-03-31T01:15:54.989Z"
|
"lastUpdated": "2026-04-01T01:14:37.127Z"
|
||||||
},
|
},
|
||||||
"lastFile": "/mnt/d/charles/five/hbrt/dynamic_syntax_test.go"
|
"lastFile": "/mnt/d/charles/five/hbrt/goroutine.go"
|
||||||
},
|
},
|
||||||
"hbrtl": {
|
"hbrtl": {
|
||||||
"phase": "do",
|
"phase": "do",
|
||||||
@@ -46,9 +46,9 @@
|
|||||||
"documents": {},
|
"documents": {},
|
||||||
"timestamps": {
|
"timestamps": {
|
||||||
"started": "2026-03-27T11:15:10.675Z",
|
"started": "2026-03-27T11:15:10.675Z",
|
||||||
"lastUpdated": "2026-03-31T01:06:49.182Z"
|
"lastUpdated": "2026-04-01T01:16:33.700Z"
|
||||||
},
|
},
|
||||||
"lastFile": "/mnt/d/charles/five/hbrtl/rawtty.go"
|
"lastFile": "/mnt/d/charles/five/hbrtl/register.go"
|
||||||
},
|
},
|
||||||
"tests": {
|
"tests": {
|
||||||
"phase": "do",
|
"phase": "do",
|
||||||
@@ -280,7 +280,7 @@
|
|||||||
"session": {
|
"session": {
|
||||||
"startedAt": "2026-03-27T06:06:49.620Z",
|
"startedAt": "2026-03-27T06:06:49.620Z",
|
||||||
"onboardingCompleted": false,
|
"onboardingCompleted": false,
|
||||||
"lastActivity": "2026-03-31T02:56:59.267Z"
|
"lastActivity": "2026-04-01T01:16:33.700Z"
|
||||||
},
|
},
|
||||||
"history": [
|
"history": [
|
||||||
{
|
{
|
||||||
@@ -5418,6 +5418,72 @@
|
|||||||
"feature": "mem",
|
"feature": "mem",
|
||||||
"phase": "do",
|
"phase": "do",
|
||||||
"action": "updated"
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:50:44.331Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:51:04.728Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:51:29.797Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:53:27.988Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:53:39.882Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T00:53:51.335Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T01:12:51.547Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T01:14:22.341Z",
|
||||||
|
"feature": "hbrt",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T01:14:37.127Z",
|
||||||
|
"feature": "hbrt",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T01:15:49.420Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"timestamp": "2026-04-01T01:16:33.700Z",
|
||||||
|
"feature": "hbrtl",
|
||||||
|
"phase": "do",
|
||||||
|
"action": "updated"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
@@ -115,7 +115,7 @@ if condition {
|
|||||||
```prg
|
```prg
|
||||||
LOCAL cCustomerName, nTotalBalance, dLastPurchase, lIsActive
|
LOCAL cCustomerName, nTotalBalance, dLastPurchase, lIsActive
|
||||||
|
|
||||||
cCustomerName := "Charles Kwon" // c = Character
|
cCustomerName := "Charles KWON" // c = Character
|
||||||
nTotalBalance := 15000.50 // n = Numeric
|
nTotalBalance := 15000.50 // n = Numeric
|
||||||
dLastPurchase := Date() // d = Date
|
dLastPurchase := Date() // d = Date
|
||||||
lIsActive := .T. // l = Logical
|
lIsActive := .T. // l = Logical
|
||||||
|
|||||||
@@ -8,6 +8,8 @@
|
|||||||
package hbrt
|
package hbrt
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
"sync"
|
"sync"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -114,6 +116,11 @@ func (v Value) AsMutex() *HbMutex {
|
|||||||
// GoLaunch spawns a new goroutine that runs a function on a new Thread.
|
// GoLaunch spawns a new goroutine that runs a function on a new Thread.
|
||||||
func (vm *VM) GoLaunch(fn func(*Thread), args []Value) {
|
func (vm *VM) GoLaunch(fn func(*Thread), args []Value) {
|
||||||
go func() {
|
go func() {
|
||||||
|
defer func() {
|
||||||
|
if r := recover(); r != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Five goroutine panic: %v\n", r)
|
||||||
|
}
|
||||||
|
}()
|
||||||
t := vm.NewThread()
|
t := vm.NewThread()
|
||||||
for _, a := range args {
|
for _, a := range args {
|
||||||
t.push(a)
|
t.push(a)
|
||||||
@@ -126,6 +133,11 @@ func (vm *VM) GoLaunch(fn func(*Thread), args []Value) {
|
|||||||
// GoLaunchBlock spawns a goroutine that evaluates a code block.
|
// GoLaunchBlock spawns a goroutine that evaluates a code block.
|
||||||
func (vm *VM) GoLaunchBlock(blk *HbBlock, args []Value) {
|
func (vm *VM) GoLaunchBlock(blk *HbBlock, args []Value) {
|
||||||
go func() {
|
go func() {
|
||||||
|
defer func() {
|
||||||
|
if r := recover(); r != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Five goroutine panic: %v\n", r)
|
||||||
|
}
|
||||||
|
}()
|
||||||
t := vm.NewThread()
|
t := vm.NewThread()
|
||||||
for _, a := range args {
|
for _, a := range args {
|
||||||
t.push(a)
|
t.push(a)
|
||||||
|
|||||||
@@ -13,14 +13,40 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
)
|
)
|
||||||
|
|
||||||
// File handle table — maps Harbour handle (int) to Go *os.File
|
// File handle table — maps Harbour handle (int) to Go *os.File
|
||||||
|
// Protected by mutex for goroutine safety.
|
||||||
var (
|
var (
|
||||||
fileHandles = map[int]*os.File{}
|
fileHandles = map[int]*os.File{}
|
||||||
nextHandle = 10 // start from 10, avoid 0-2 (stdin/out/err)
|
nextHandle = 10 // start from 10, avoid 0-2 (stdin/out/err)
|
||||||
|
fileHandlesMu sync.Mutex
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// File handle helpers (thread-safe)
|
||||||
|
func allocHandle(f *os.File) int {
|
||||||
|
fileHandlesMu.Lock()
|
||||||
|
defer fileHandlesMu.Unlock()
|
||||||
|
h := nextHandle
|
||||||
|
nextHandle++
|
||||||
|
fileHandles[h] = f
|
||||||
|
return h
|
||||||
|
}
|
||||||
|
|
||||||
|
func getHandle(h int) (*os.File, bool) {
|
||||||
|
fileHandlesMu.Lock()
|
||||||
|
defer fileHandlesMu.Unlock()
|
||||||
|
f, ok := fileHandles[h]
|
||||||
|
return f, ok
|
||||||
|
}
|
||||||
|
|
||||||
|
func removeHandle(h int) {
|
||||||
|
fileHandlesMu.Lock()
|
||||||
|
defer fileHandlesMu.Unlock()
|
||||||
|
delete(fileHandles, h)
|
||||||
|
}
|
||||||
|
|
||||||
// FOPEN(cFileName [, nMode]) → nHandle | -1
|
// FOPEN(cFileName [, nMode]) → nHandle | -1
|
||||||
// nMode: 0=read, 1=write, 2=readwrite
|
// nMode: 0=read, 1=write, 2=readwrite
|
||||||
func FOpen(t *hbrt.Thread) {
|
func FOpen(t *hbrt.Thread) {
|
||||||
@@ -53,9 +79,7 @@ func FOpen(t *hbrt.Thread) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
h := nextHandle
|
h := allocHandle(f)
|
||||||
nextHandle++
|
|
||||||
fileHandles[h] = f
|
|
||||||
SetFError(0)
|
SetFError(0)
|
||||||
t.RetInt(int64(h))
|
t.RetInt(int64(h))
|
||||||
}
|
}
|
||||||
@@ -74,9 +98,7 @@ func FCreate(t *hbrt.Thread) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
h := nextHandle
|
h := allocHandle(f)
|
||||||
nextHandle++
|
|
||||||
fileHandles[h] = f
|
|
||||||
SetFError(0)
|
SetFError(0)
|
||||||
t.RetInt(int64(h))
|
t.RetInt(int64(h))
|
||||||
}
|
}
|
||||||
@@ -87,14 +109,14 @@ func FClose(t *hbrt.Thread) {
|
|||||||
defer t.EndProc()
|
defer t.EndProc()
|
||||||
|
|
||||||
h := t.Local(1).AsInt()
|
h := t.Local(1).AsInt()
|
||||||
f, ok := fileHandles[h]
|
f, ok := getHandle(h)
|
||||||
if !ok {
|
if !ok {
|
||||||
SetFError(6) // invalid handle
|
SetFError(6) // invalid handle
|
||||||
t.RetBool(false)
|
t.RetBool(false)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
err := f.Close()
|
err := f.Close()
|
||||||
delete(fileHandles, h)
|
removeHandle(h)
|
||||||
SetFError(0)
|
SetFError(0)
|
||||||
t.RetBool(err == nil)
|
t.RetBool(err == nil)
|
||||||
}
|
}
|
||||||
@@ -107,7 +129,7 @@ func FRead(t *hbrt.Thread) {
|
|||||||
h := t.Local(1).AsInt()
|
h := t.Local(1).AsInt()
|
||||||
nBytes := t.Local(3).AsInt()
|
nBytes := t.Local(3).AsInt()
|
||||||
|
|
||||||
f, ok := fileHandles[h]
|
f, ok := getHandle(h)
|
||||||
if !ok {
|
if !ok {
|
||||||
SetFError(6)
|
SetFError(6)
|
||||||
t.RetInt(0)
|
t.RetInt(0)
|
||||||
@@ -139,7 +161,7 @@ func FWrite(t *hbrt.Thread) {
|
|||||||
h := t.Local(1).AsInt()
|
h := t.Local(1).AsInt()
|
||||||
data := t.Local(2).AsString()
|
data := t.Local(2).AsString()
|
||||||
|
|
||||||
f, ok := fileHandles[h]
|
f, ok := getHandle(h)
|
||||||
if !ok {
|
if !ok {
|
||||||
SetFError(6)
|
SetFError(6)
|
||||||
t.RetInt(0)
|
t.RetInt(0)
|
||||||
@@ -177,7 +199,7 @@ func FSeek(t *hbrt.Thread) {
|
|||||||
origin = t.Local(3).AsInt()
|
origin = t.Local(3).AsInt()
|
||||||
}
|
}
|
||||||
|
|
||||||
f, ok := fileHandles[h]
|
f, ok := getHandle(h)
|
||||||
if !ok {
|
if !ok {
|
||||||
SetFError(6)
|
SetFError(6)
|
||||||
t.RetInt(0)
|
t.RetInt(0)
|
||||||
|
|||||||
@@ -33,9 +33,9 @@ func RegisterRTL(vm *hbrt.VM) {
|
|||||||
hbrt.Sym("UPPER", hbrt.FsPublic, Upper),
|
hbrt.Sym("UPPER", hbrt.FsPublic, Upper),
|
||||||
hbrt.Sym("LOWER", hbrt.FsPublic, Lower),
|
hbrt.Sym("LOWER", hbrt.FsPublic, Lower),
|
||||||
hbrt.Sym("ALLTRIM", hbrt.FsPublic, AllTrim),
|
hbrt.Sym("ALLTRIM", hbrt.FsPublic, AllTrim),
|
||||||
hbrt.Sym("LTRIM", hbrt.FsPublic, AllTrim), // simplified alias
|
hbrt.Sym("LTRIM", hbrt.FsPublic, LTrim),
|
||||||
hbrt.Sym("RTRIM", hbrt.FsPublic, AllTrim), // simplified alias
|
hbrt.Sym("RTRIM", hbrt.FsPublic, RTrim),
|
||||||
hbrt.Sym("TRIM", hbrt.FsPublic, AllTrim), // simplified alias
|
hbrt.Sym("TRIM", hbrt.FsPublic, RTrim), // TRIM = RTRIM in Harbour
|
||||||
hbrt.Sym("SPACE", hbrt.FsPublic, Space),
|
hbrt.Sym("SPACE", hbrt.FsPublic, Space),
|
||||||
hbrt.Sym("PADR", hbrt.FsPublic, PadR),
|
hbrt.Sym("PADR", hbrt.FsPublic, PadR),
|
||||||
hbrt.Sym("PADL", hbrt.FsPublic, PadL),
|
hbrt.Sym("PADL", hbrt.FsPublic, PadL),
|
||||||
|
|||||||
@@ -202,6 +202,32 @@ func AllTrim(t *hbrt.Thread) {
|
|||||||
t.RetValue()
|
t.RetValue()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// LTrim trims leading spaces only. Harbour: LTRIM(cString) → cString
|
||||||
|
func LTrim(t *hbrt.Thread) {
|
||||||
|
t.Frame(1, 0)
|
||||||
|
defer t.EndProc()
|
||||||
|
v := t.Local(1)
|
||||||
|
if v.IsString() {
|
||||||
|
t.PushString(strings.TrimLeft(v.AsString(), " "))
|
||||||
|
} else {
|
||||||
|
t.PushString("")
|
||||||
|
}
|
||||||
|
t.RetValue()
|
||||||
|
}
|
||||||
|
|
||||||
|
// RTrim trims trailing spaces only. Harbour: RTRIM(cString) / TRIM(cString)
|
||||||
|
func RTrim(t *hbrt.Thread) {
|
||||||
|
t.Frame(1, 0)
|
||||||
|
defer t.EndProc()
|
||||||
|
v := t.Local(1)
|
||||||
|
if v.IsString() {
|
||||||
|
t.PushString(strings.TrimRight(v.AsString(), " "))
|
||||||
|
} else {
|
||||||
|
t.PushString("")
|
||||||
|
}
|
||||||
|
t.RetValue()
|
||||||
|
}
|
||||||
|
|
||||||
// Space returns a string of n spaces.
|
// Space returns a string of n spaces.
|
||||||
// Harbour: Space(nCount) → cString
|
// Harbour: Space(nCount) → cString
|
||||||
func Space(t *hbrt.Thread) {
|
func Space(t *hbrt.Thread) {
|
||||||
|
|||||||
Reference in New Issue
Block a user