Fix hang on second COM port open: CLI deadlock, MOVEABLE buffers, stale UART

setque (called by USER.EXE after every inicom) was running GlobalAlloc and
GlobalFree with interrupts disabled.  On the second open the global heap
could need compaction or DPMI selector work, deadlocking under CLI.
Restructured to allocate new buffers first, swap pointers under CLI, then
free old buffers after STI.

Ring buffers changed from GMEM_MOVEABLE to GMEM_FIXED so addresses are
inherently stable at interrupt time.  Added rxBufH/txBufH handle fields
to PortStateT, eliminating the fragile GlobalHandle(SELECTOROF()) recovery
in freeBuffers.

trmcom now resets FIFOs and drains all pending UART conditions (LSR, MSR,
RBR, IIR) before unhooking the ISR, leaving hardware clean for reopen.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Scott Duensing 2026-02-28 20:06:04 -06:00
parent b6f08a3150
commit 5dd464eb18
2 changed files with 101 additions and 47 deletions

View file

@ -185,7 +185,7 @@ void applyLineParams(PortStateT *port, uint8_t byteSize, uint8_t pari
static uint32_t cbrToBaud(uint16_t cbr); static uint32_t cbrToBaud(uint16_t cbr);
int16_t detect16550(uint16_t baseAddr); int16_t detect16550(uint16_t baseAddr);
void enableFifo(PortStateT *port); void enableFifo(PortStateT *port);
static int16_t freeBuffers(PortStateT *port); static void freeBuffers(PortStateT *port);
static int16_t initBuffers(PortStateT *port, uint16_t rxSz, uint16_t txSz); static int16_t initBuffers(PortStateT *port, uint16_t rxSz, uint16_t txSz);
static void initPortState(PortStateT *port, int16_t commId); static void initPortState(PortStateT *port, int16_t commId);
void primeTx(PortStateT *port); void primeTx(PortStateT *port);
@ -763,29 +763,21 @@ int16_t FAR PASCAL _export enableNotification(int16_t commId, HWND hwnd, int16_t
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// freeBuffers - Free ring buffers for a port // freeBuffers - Free ring buffers for a port
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
static int16_t freeBuffers(PortStateT *port) static void freeBuffers(PortStateT *port)
{ {
HGLOBAL hMem; if (port->rxBufH) {
GlobalUnlock(port->rxBufH);
if (port->rxBuf) { GlobalFree(port->rxBufH);
hMem = (HGLOBAL)LOWORD(GlobalHandle(SELECTOROF(port->rxBuf))); port->rxBufH = 0;
if (hMem) { port->rxBuf = NULL;
GlobalUnlock(hMem);
GlobalFree(hMem);
}
port->rxBuf = NULL;
} }
if (port->txBuf) { if (port->txBufH) {
hMem = (HGLOBAL)LOWORD(GlobalHandle(SELECTOROF(port->txBuf))); GlobalUnlock(port->txBufH);
if (hMem) { GlobalFree(port->txBufH);
GlobalUnlock(hMem); port->txBufH = 0;
GlobalFree(hMem); port->txBuf = NULL;
}
port->txBuf = NULL;
} }
return 0;
} }
@ -977,33 +969,39 @@ int16_t FAR PASCAL _export inicom(DCB FAR *dcb)
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// initBuffers - Allocate ring buffers via GlobalAlloc // initBuffers - Allocate ring buffers via GlobalAlloc
//
// Uses GMEM_FIXED so buffer addresses are stable at interrupt time
// without requiring GlobalLock. Saves handles in PortStateT for
// clean freeing (no GlobalHandle recovery hack).
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
static int16_t initBuffers(PortStateT *port, uint16_t rxSz, uint16_t txSz) static int16_t initBuffers(PortStateT *port, uint16_t rxSz, uint16_t txSz)
{ {
HGLOBAL hRx; HGLOBAL hRx;
HGLOBAL hTx; HGLOBAL hTx;
hRx = GlobalAlloc(GMEM_MOVEABLE | GMEM_ZEROINIT, (DWORD)rxSz); hRx = GlobalAlloc(GMEM_FIXED | GMEM_ZEROINIT, (DWORD)rxSz);
if (!hRx) { if (!hRx) {
return -1; return -1;
} }
hTx = GlobalAlloc(GMEM_MOVEABLE | GMEM_ZEROINIT, (DWORD)txSz); hTx = GlobalAlloc(GMEM_FIXED | GMEM_ZEROINIT, (DWORD)txSz);
if (!hTx) { if (!hTx) {
GlobalFree(hRx); GlobalFree(hRx);
return -1; return -1;
} }
port->rxBuf = (uint8_t FAR *)GlobalLock(hRx); port->rxBufH = hRx;
port->rxSize = rxSz; port->rxBuf = (uint8_t FAR *)GlobalLock(hRx);
port->rxHead = 0; port->rxSize = rxSz;
port->rxTail = 0; port->rxHead = 0;
port->rxTail = 0;
port->rxCount = 0; port->rxCount = 0;
port->txBuf = (uint8_t FAR *)GlobalLock(hTx); port->txBufH = hTx;
port->txSize = txSz; port->txBuf = (uint8_t FAR *)GlobalLock(hTx);
port->txHead = 0; port->txSize = txSz;
port->txTail = 0; port->txHead = 0;
port->txTail = 0;
port->txCount = 0; port->txCount = 0;
return 0; return 0;
@ -1411,23 +1409,68 @@ int16_t FAR PASCAL _export setque(int16_t commId, int16_t rxSz, int16_t txSz)
return -1; return -1;
} }
_disable(); // Allocate new buffers BEFORE disabling interrupts.
freeBuffers(port); // GlobalAlloc/GlobalFree can deadlock under CLI if the heap
if (initBuffers(port, (uint16_t)rxSz, (uint16_t)txSz) != 0) { // needs compaction or DPMI services.
{
HGLOBAL newRxH;
HGLOBAL newTxH;
uint8_t FAR *newRxBuf;
uint8_t FAR *newTxBuf;
HGLOBAL oldRxH;
HGLOBAL oldTxH;
newRxH = GlobalAlloc(GMEM_FIXED | GMEM_ZEROINIT, (DWORD)(uint16_t)rxSz);
if (!newRxH) {
dbgStr("KPCOMM: setque IE_MEMORY\r\n");
return IE_MEMORY;
}
newTxH = GlobalAlloc(GMEM_FIXED | GMEM_ZEROINIT, (DWORD)(uint16_t)txSz);
if (!newTxH) {
GlobalFree(newRxH);
dbgStr("KPCOMM: setque IE_MEMORY\r\n");
return IE_MEMORY;
}
newRxBuf = (uint8_t FAR *)GlobalLock(newRxH);
newTxBuf = (uint8_t FAR *)GlobalLock(newTxH);
// Swap buffers with interrupts disabled -- ISR sees
// consistent pointers at all times
_disable();
oldRxH = port->rxBufH;
oldTxH = port->txBufH;
port->rxBufH = newRxH;
port->rxBuf = newRxBuf;
port->rxSize = (uint16_t)rxSz;
port->rxHead = 0;
port->rxTail = 0;
port->rxCount = 0;
port->txBufH = newTxH;
port->txBuf = newTxBuf;
port->txSize = (uint16_t)txSz;
port->txHead = 0;
port->txTail = 0;
port->txCount = 0;
port->comDeb.qInSize = (uint16_t)rxSz;
port->comDeb.qInCount = 0;
port->comDeb.qInHead = 0;
port->comDeb.qInTail = 0;
port->comDeb.qOutSize = (uint16_t)txSz;
port->comDeb.qOutCount = 0;
port->comDeb.qOutHead = 0;
port->comDeb.qOutTail = 0;
_enable(); _enable();
dbgStr("KPCOMM: setque IE_MEMORY\r\n");
return IE_MEMORY; // Free old buffers AFTER re-enabling interrupts
if (oldRxH) {
GlobalUnlock(oldRxH);
GlobalFree(oldRxH);
}
if (oldTxH) {
GlobalUnlock(oldTxH);
GlobalFree(oldTxH);
}
} }
// Sync ComDEB queue sizes
port->comDeb.qInSize = port->rxSize;
port->comDeb.qInCount = 0;
port->comDeb.qInHead = 0;
port->comDeb.qInTail = 0;
port->comDeb.qOutSize = port->txSize;
port->comDeb.qOutCount = 0;
port->comDeb.qOutHead = 0;
port->comDeb.qOutTail = 0;
_enable();
dbgStr("KPCOMM: setque OK\r\n"); dbgStr("KPCOMM: setque OK\r\n");
return 0; return 0;
@ -1590,14 +1633,23 @@ int16_t FAR PASCAL _export trmcom(int16_t commId)
// Disable UART interrupts // Disable UART interrupts
_outp(port->baseAddr + UART_IER, 0); _outp(port->baseAddr + UART_IER, 0);
// Disable FIFOs // Reset FIFOs to drain pending data, then disable
if (port->is16550) { if (port->is16550) {
_outp(port->baseAddr + UART_FCR, FCR_ENABLE | FCR_RX_RESET | FCR_TX_RESET);
_outp(port->baseAddr + UART_FCR, 0); _outp(port->baseAddr + UART_FCR, 0);
} }
// Drop DTR, RTS, OUT2 // Drop DTR, RTS, OUT2
_outp(port->baseAddr + UART_MCR, 0); _outp(port->baseAddr + UART_MCR, 0);
// Drain all pending UART conditions so the hardware is clean
// for the next open. Each read clears the corresponding
// interrupt source inside the UART.
_inp(port->baseAddr + UART_LSR);
_inp(port->baseAddr + UART_MSR);
_inp(port->baseAddr + UART_RBR);
_inp(port->baseAddr + UART_IIR);
// Unhook ISR // Unhook ISR
unhookIsr(port); unhookIsr(port);

View file

@ -336,13 +336,15 @@ typedef struct {
uint8_t fifoEnabled; // FIFO enabled (COMnFIFO setting) uint8_t fifoEnabled; // FIFO enabled (COMnFIFO setting)
uint8_t fifoTrigger; // RX FIFO trigger FCR bits (FCR_TRIG_*) uint8_t fifoTrigger; // RX FIFO trigger FCR bits (FCR_TRIG_*)
// Ring buffers (GlobalAlloc'd) // Ring buffers (GlobalAlloc'd GMEM_FIXED)
HGLOBAL rxBufH; // Receive buffer handle
uint8_t FAR *rxBuf; // Receive ring buffer uint8_t FAR *rxBuf; // Receive ring buffer
uint16_t rxSize; // Buffer size uint16_t rxSize; // Buffer size
uint16_t rxHead; // Write position (ISR writes) uint16_t rxHead; // Write position (ISR writes)
uint16_t rxTail; // Read position (app reads) uint16_t rxTail; // Read position (app reads)
uint16_t rxCount; // Bytes in buffer uint16_t rxCount; // Bytes in buffer
HGLOBAL txBufH; // Transmit buffer handle
uint8_t FAR *txBuf; // Transmit ring buffer uint8_t FAR *txBuf; // Transmit ring buffer
uint16_t txSize; // Buffer size uint16_t txSize; // Buffer size
uint16_t txHead; // Write position (app writes) uint16_t txHead; // Write position (app writes)