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

Death due to HANDLE_FREE if "ec +all" is active and an application is closed #605

Open
mgroeber9110 opened this issue Jul 16, 2024 · 1 comment

Comments

@mgroeber9110
Copy link
Contributor

mgroeber9110 commented Jul 16, 2024

This is a somewhat artifical situation, but I still wonder why this has not been caught befor by other projects using PC/GEOS.

I have experimented with the ec +all command in Swat (which enables some app-specific debugging and stress-testing, such as forcing ThreadBorrowStackSpace to always borrow, even if there is theoretically still enough space).

However, this makes most apps crash with this backtrace when closing them:

  1: near FatalError(), bootBoot.asm:870
  2:  far AppFatalError(), bootBoot.asm:870
* 3: near ECCheckMemHandleNS(), heapErrorCheck.asm:233
  4:  far ECCheckMemHandleNSFar(), heapErrorCheck.asm:219
  5:  far ECCheckGeodeHandle(), geodesErrorCheck.asm:184
  6: near MemAllocLow(), heapLow.asm:491
  7: near MemAllocSetOwner(), heapHigh.asm:222
  8: near MemAlloc(), heapHigh.asm:213
  9:  far ThreadBorrowStackSpace(), threadThread.asm:2141
 10: near FullLockNoReload(), heapLow.asm:176
 11:  far FarFullLockNoReload(), heapLow.asm:156
 12: near FileLockPath(), filePath.asm:4010
 13: near FP_PathLockDS(), filePath.asm:3962
 14:  far FileDeletePathStack(), filePath.asm:4717
 16:  far ThreadDestroy(), threadThread.asm:782

The reason appears to be that ThreadDestroy calls FileDeletePathStack after releasing the Geode handle - it cannot do it before, because the thread exit handlers of some libraries still need the current path to work correctly. However, this means that ThreadBorrowStackSpace no longer has a Geode handle to allocated the new stack from, causing the fatal error.

I don't see any obvious signs that this code path has been changed recently, so I am a bit surprised that this has not caught out other users of ec +app. Perhaps EC flags are a feature that has not been widely used, at least I had completely forgotten about it (even though I now remember having used it in the past...).

@rabe-soft
Copy link
Contributor

I can confirm this. I have used ec all very often (more often, I use ec ALL) and mostly the app crashes while closing. Even in the old SDK. So, I use 'ec none' to avoid this. The behavior is so old that it never occurred to me to make an issue out of it. :-)

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

No branches or pull requests

2 participants