search

bluewaysw / pcgeos

#FreeGEOS source codes. The offical home of the PC/GEOS operating system technology. For personal computing fans. For all developers and assembly lovers. For YOU!
Apache License 2.0
757 stars 87 forks source link

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

Open mgroeber9110 opened 3 months ago

mgroeber9110 commented 3 months ago

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 commented 3 months ago

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. :-)