Author: cfinck Date: Tue Jul 7 10:30:30 2015 New Revision: 68364
URL: http://svn.reactos.org/svn/reactos?rev=68364&view=rev Log: [LOCALMON] - Bugfix: LocalmonClosePort must only free the memory for virtual file ports. Other port entries are reused. - Bugfix: Set the port type to PortType_OtherLPT in all conditions that ít's not a physical LPT port. - Synchronize access to the port lists. - Keep a list of open Xcv handles as well to properly close them on shutdown. - Add traces to all monitor functions for easier debugging. (requires DEBUGCHANNEL=+localmon environment variable)
Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c
Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c URL: http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/reac... ============================================================================== --- branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c [iso-8859-1] (original) +++ branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c [iso-8859-1] Tue Jul 7 10:30:30 2015 @@ -67,6 +67,9 @@ { PLOCALMON_HANDLE pLocalmon; PLOCALMON_PORT pPort; + PLOCALMON_XCV pXcv; + + TRACE("LocalmonShutdown(%p)\n", hMonitor);
pLocalmon = (PLOCALMON_HANDLE)hMonitor;
@@ -77,15 +80,24 @@ LocalmonClosePort((HANDLE)pPort); }
- // Now close all regular ports and remove them from the list. - while (!IsListEmpty(&pLocalmon->Ports)) + // Do the same for the open Xcv ports. + while (!IsListEmpty(&pLocalmon->XcvHandles)) { - pPort = CONTAINING_RECORD(&pLocalmon->Ports.Flink, LOCALMON_PORT, Entry); - RemoveEntryList(&pPort->Entry); - LocalmonClosePort((HANDLE)pPort); + pXcv = CONTAINING_RECORD(&pLocalmon->XcvHandles.Flink, LOCALMON_XCV, Entry); + LocalmonXcvClosePort((HANDLE)pXcv); }
- // Finally free the memory for the LOCALMON_HANDLE structure itself. + // Now close all registry ports, remove them from the list and free their memory. + while (!IsListEmpty(&pLocalmon->RegistryPorts)) + { + pPort = CONTAINING_RECORD(&pLocalmon->RegistryPorts.Flink, LOCALMON_PORT, Entry); + LocalmonClosePort((HANDLE)pPort); + RemoveEntryList(&pPort->Entry); + DllFreeSplMem(pPort); + } + + // Finally clean the LOCALMON_HANDLE structure itself. + DeleteCriticalSection(&pLocalmon->Section); DllFreeSplMem(pLocalmon); }
@@ -102,10 +114,14 @@ PLOCALMON_HANDLE pLocalmon; PLOCALMON_PORT pPort = NULL;
+ TRACE("InitializePrintMonitor2(%p, %p)\n", pMonitorInit, phMonitor); + // Create a new LOCALMON_HANDLE structure. pLocalmon = DllAllocSplMem(sizeof(LOCALMON_HANDLE)); + InitializeCriticalSection(&pLocalmon->Section); InitializeListHead(&pLocalmon->FilePorts); - InitializeListHead(&pLocalmon->Ports); + InitializeListHead(&pLocalmon->RegistryPorts); + InitializeListHead(&pLocalmon->XcvHandles);
// The Local Spooler Port Monitor doesn't need to care about the given registry key and functions. // Instead it uses a well-known registry key for getting its information about local ports. Open this one. @@ -136,6 +152,7 @@ goto Cleanup; }
+ pPort->pLocalmon = pLocalmon; pPort->hFile = INVALID_HANDLE_VALUE; pPort->pwszPortName = (PWSTR)((PBYTE)pPort + sizeof(LOCALMON_PORT));
@@ -156,7 +173,7 @@ }
// Add it to the list. - InsertTailList(&pLocalmon->Ports, &pPort->Entry); + InsertTailList(&pLocalmon->RegistryPorts, &pPort->Entry);
// Don't let the cleanup routine free this. pPort = NULL;
Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c URL: http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/reac... ============================================================================== --- branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c [iso-8859-1] (original) +++ branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c [iso-8859-1] Tue Jul 7 10:30:30 2015 @@ -49,7 +49,7 @@ /** * @name _ClosePortHandles * - * Closes a port of any type if its opened. + * Closes a port of any type if it's open. * Removes any saved mapping or existing definition of a NONSPOOLED device mapping. * * @param pPort @@ -305,7 +305,7 @@ PLIST_ENTRY pEntry; PLOCALMON_PORT pPort;
- for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry = pEntry->Flink) + for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry != &pLocalmon->RegistryPorts; pEntry = pEntry->Flink) { pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
@@ -329,7 +329,7 @@ PORT_INFO_1W PortInfo1;
// Count the required buffer size and the number of datatypes. - for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry = pEntry->Flink) + for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry != &pLocalmon->RegistryPorts; pEntry = pEntry->Flink) { pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
@@ -350,7 +350,7 @@ pPortString = pPorts + dwPortCount * sizeof(PORT_INFO_1W);
// Copy over all ports. - for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry = pEntry->Flink) + for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry != &pLocalmon->RegistryPorts; pEntry = pEntry->Flink) { pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
@@ -385,7 +385,7 @@ PORT_INFO_2W PortInfo2;
// Count the required buffer size and the number of datatypes. - for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry = pEntry->Flink) + for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry != &pLocalmon->RegistryPorts; pEntry = pEntry->Flink) { pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
@@ -406,7 +406,7 @@ pPortString = pPorts + dwPortCount * sizeof(PORT_INFO_2W);
// Copy over all ports. - for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry = pEntry->Flink) + for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry != &pLocalmon->RegistryPorts; pEntry = pEntry->Flink) { pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
@@ -472,32 +472,35 @@ BOOL WINAPI LocalmonClosePort(HANDLE hPort) { - PLOCALMON_PORT pPort; + PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort; + + TRACE("LocalmonClosePort(%p)\n", hPort);
// Sanity checks - if (!hPort) + if (!pPort) { SetLastError(ERROR_INVALID_PARAMETER); return FALSE; }
- pPort = (PLOCALMON_PORT)hPort; - - if (pPort->PortType == PortType_FILE) - { - // Remove it from the list of virtual file ports. - RemoveEntryList(&pPort->Entry); - } - // Close the file handle, free memory for pwszMapping and delete any NONSPOOLED port. _ClosePortHandles(pPort);
// Close any open printer handle. if (pPort->hPrinter) + { ClosePrinter(pPort->hPrinter); - - // Finally free the memory for the port itself. - DllFreeSplMem(pPort); + pPort->hPrinter = NULL; + } + + // Free virtual FILE ports which were created in LocalmonOpenPort. + if (pPort->PortType == PortType_FILE) + { + EnterCriticalSection(&pPort->pLocalmon->Section); + RemoveEntryList(&pPort->Entry); + DllFreeSplMem(pPort); + LeaveCriticalSection(&pPort->pLocalmon->Section); + }
SetLastError(ERROR_SUCCESS); return TRUE; @@ -507,6 +510,8 @@ LocalmonEndDocPort(HANDLE hPort) { PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort; + + TRACE("LocalmonEndDocPort(%p)\n", hPort);
// Sanity checks if (!pPort) @@ -539,9 +544,12 @@ LocalmonEnumPorts(HANDLE hMonitor, PWSTR pName, DWORD Level, PBYTE pPorts, DWORD cbBuf, PDWORD pcbNeeded, PDWORD pcReturned) { DWORD dwErrorCode; + PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor; + + TRACE("LocalmonEnumPorts(%p, %S, %lu, %p, %lu, %p, %p)\n", hMonitor, pName, Level, pPorts, cbBuf, pcbNeeded, pcReturned);
// Sanity checks - if (!hMonitor || (cbBuf && !pPorts) || !pcbNeeded || !pcReturned) + if (!pLocalmon || (cbBuf && !pPorts) || !pcbNeeded || !pcReturned) { dwErrorCode = ERROR_INVALID_PARAMETER; goto Cleanup; @@ -557,11 +565,15 @@ *pcbNeeded = 0; *pcReturned = 0;
+ EnterCriticalSection(&pLocalmon->Section); + // The function behaves differently for each level. if (Level == 1) - dwErrorCode = _LocalmonEnumPortsLevel1((PLOCALMON_HANDLE)hMonitor, pPorts, cbBuf, pcbNeeded, pcReturned); + dwErrorCode = _LocalmonEnumPortsLevel1(pLocalmon, pPorts, cbBuf, pcbNeeded, pcReturned); else if (Level == 2) - dwErrorCode = _LocalmonEnumPortsLevel2((PLOCALMON_HANDLE)hMonitor, pPorts, cbBuf, pcbNeeded, pcReturned); + dwErrorCode = _LocalmonEnumPortsLevel2(pLocalmon, pPorts, cbBuf, pcbNeeded, pcReturned); + + LeaveCriticalSection(&pLocalmon->Section);
Cleanup: SetLastError(dwErrorCode); @@ -569,7 +581,7 @@ }
/* - * @name LocalmonSetPortTimeOuts + * @name LocalmonGetPrinterDataFromPort * * Performs a DeviceIoControl call for the given port. * @@ -608,6 +620,8 @@ DWORD dwErrorCode; PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+ TRACE("LocalmonGetPrinterDataFromPort(%p, %lu, %p, %p, %lu, %p, %lu, %p)\n", hPort, ControlID, pValueName, lpInBuffer, cbInBuffer, lpOutBuffer, cbOutBuffer, lpcbReturned); + // Sanity checks if (!pPort || !ControlID || !lpcbReturned) { @@ -660,6 +674,8 @@ PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor; PLOCALMON_PORT pPort;
+ TRACE("LocalmonOpenPort(%p, %S, %p)\n", hMonitor, pName, pHandle); + // Sanity checks if (!pLocalmon || !pName || !pHandle) { @@ -672,18 +688,23 @@ { // For FILE:, we create a virtual port for each request. pPort = DllAllocSplMem(sizeof(LOCALMON_PORT)); + pPort->pLocalmon = pLocalmon; pPort->PortType = PortType_FILE; pPort->hFile = INVALID_HANDLE_VALUE;
// Add it to the list of file ports. + EnterCriticalSection(&pLocalmon->Section); InsertTailList(&pLocalmon->FilePorts, &pPort->Entry); } else { + EnterCriticalSection(&pLocalmon->Section); + // Check if the port name is valid. pPort = _FindPort(pLocalmon, pName); if (!pPort) { + LeaveCriticalSection(&pLocalmon->Section); dwErrorCode = ERROR_UNKNOWN_PORT; goto Cleanup; } @@ -693,6 +714,9 @@ // The others are only opened per job in StartDocPort. if (_wcsnicmp(pName, L"LPT", 3) == 0) { + // Treat all ports as other LPT ports until we can definitely say that it's a physical one. + pPort->PortType = PortType_OtherLPT; + // Try to create a NONSPOOLED port and open it. if (_CreateNonspooledPort(pPort)) { @@ -705,13 +729,13 @@ } else { - // This is no physical port, so don't keep its handle open all the time. + // This is no physical port, so don't keep its handle open. _ClosePortHandles(pPort); - pPort->PortType = PortType_OtherLPT; } } else if (GetLastError() != ERROR_SUCCESS) { + LeaveCriticalSection(&pLocalmon->Section); dwErrorCode = GetLastError(); goto Cleanup; } @@ -722,6 +746,8 @@ pPort->PortType = PortType_PhysicalCOM; } } + + LeaveCriticalSection(&pLocalmon->Section);
// Return our fetched LOCALMON_PORT structure in the handle. *pHandle = (PHANDLE)pPort; @@ -757,6 +783,8 @@ DWORD dwErrorCode; PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+ TRACE("LocalmonSetPortTimeOuts(%p, %p, %lu)\n", hPort, lpCTO, Reserved); + // Sanity checks if (!pPort || !lpCTO) { @@ -809,6 +837,8 @@ DWORD dwErrorCode; PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+ TRACE("LocalmonReadPort(%p, %p, %lu, %p)\n", hPort, pBuffer, cbBuffer, pcbRead); + // Sanity checks if (!pPort || (cbBuffer && !pBuffer) || !pcbRead) { @@ -859,6 +889,8 @@ DWORD dwErrorCode; PDOC_INFO_1W pDocInfo1 = (PDOC_INFO_1W)pDocInfo; // DOC_INFO_1W is the least common denominator for both DOC_INFO levels. PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort; + + TRACE("LocalmonStartDocPort(%p, %S, %lu, %lu, %p)\n", hPort, pPrinterName, JobId, Level, pDocInfo);
// Sanity checks if (!pPort || !pPrinterName || (pPort->PortType == PortType_FILE && (!pDocInfo1 || !pDocInfo1->pOutputFile || !*pDocInfo1->pOutputFile))) @@ -954,6 +986,8 @@ DWORD dwErrorCode; PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+ TRACE("LocalmonWritePort(%p, %p, %lu, %p)\n", hPort, pBuffer, cbBuf, pcbWritten); + // Sanity checks if (!pPort || (cbBuf && !pBuffer) || !pcbWritten) {
Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h URL: http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/reac... ============================================================================== --- branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h [iso-8859-1] (original) +++ branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h [iso-8859-1] Tue Jul 7 10:30:30 2015 @@ -29,6 +29,19 @@
// Structures /** + * Describes the monitor handle returned by InitializePrintMonitor2. + * Manages all available ports in this instance. + */ +typedef struct _LOCALMON_HANDLE +{ + CRITICAL_SECTION Section; /** Critical Section for modifying or reading the ports. */ + LIST_ENTRY FilePorts; /** Virtual ports created for every document that's redirected to an output file. */ + LIST_ENTRY RegistryPorts; /** COM, FILE: and LPT ports loaded from the local registry. */ + LIST_ENTRY XcvHandles; /** Xcv handles created with LocalmonXcvOpenPort. */ +} +LOCALMON_HANDLE, *PLOCALMON_HANDLE; + +/** * Describes the port handle returned by LocalmonOpenPort. * Manages a legacy port (COM/LPT) or virtual FILE: port for printing as well as its associated printer and job. */ @@ -46,21 +59,11 @@ DWORD dwJobID; /** ID of the printing job we are processing (for later reporting progress using SetJobW). */ HANDLE hFile; /** Handle to the opened port or INVALID_HANDLE_VALUE if it isn't currently opened. */ HANDLE hPrinter; /** Handle to the printer for the job on this port (for using SetJobW). */ + PLOCALMON_HANDLE pLocalmon; /** Pointer to the parent LOCALMON_HANDLE structure. */ PWSTR pwszMapping; /** The current mapping of the DOS Device corresponding to this port at the time _CreateNonspooledPort has been called. */ PWSTR pwszPortName; /** The name of this port including the trailing colon. Empty for virtual file ports. */ } LOCALMON_PORT, *PLOCALMON_PORT; - -/** - * Describes the monitor handle returned by InitializePrintMonitor2. - * Manages all available ports in this instance. - */ -typedef struct _LOCALMON_HANDLE -{ - LIST_ENTRY FilePorts; /** Virtual ports created for every document that's redirected to an output file. */ - LIST_ENTRY Ports; /** Ports found on the system (except for FILE:) */ -} -LOCALMON_HANDLE, *PLOCALMON_HANDLE;
/** * Describes the Xcv handle returned by LocalmonXcvOpenPort. @@ -68,7 +71,9 @@ */ typedef struct _LOCALMON_XCV { + LIST_ENTRY Entry; ACCESS_MASK GrantedAccess; + PLOCALMON_HANDLE pLocalmon; PWSTR pwszObject; } LOCALMON_XCV, *PLOCALMON_XCV;
Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c URL: http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/reac... ============================================================================== --- branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c [iso-8859-1] (original) +++ branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c [iso-8859-1] Tue Jul 7 10:30:30 2015 @@ -345,15 +345,21 @@ BOOL WINAPI LocalmonXcvClosePort(HANDLE hXcv) { - // Sanity checks - if (!hXcv) + PLOCALMON_XCV pXcv = (PLOCALMON_XCV)hXcv; + + TRACE("LocalmonXcvClosePort(%p)\n", hXcv); + + // Sanity checks + if (!pXcv) { SetLastError(ERROR_INVALID_PARAMETER); return FALSE; }
- // Free the memory. - DllFreeSplMem(hXcv); + // Remove it from the list and free the memory. + RemoveEntryList(&pXcv->Entry); + DllFreeSplMem(pXcv); + SetLastError(ERROR_SUCCESS); return TRUE; } @@ -361,6 +367,8 @@ DWORD WINAPI LocalmonXcvDataPort(HANDLE hXcv, PCWSTR pszDataName, PBYTE pInputData, DWORD cbInputData, PBYTE pOutputData, DWORD cbOutputData, PDWORD pcbOutputNeeded) { + TRACE("LocalmonXcvDataPort(%p, %S, %p, %lu, %p, %lu, %p)\n", hXcv, pszDataName, pInputData, cbInputData, pOutputData, cbOutputData, pcbOutputNeeded); + // Sanity checks if (!pszDataName) return ERROR_INVALID_PARAMETER; @@ -401,10 +409,13 @@ { DWORD cbObject = 0; DWORD dwErrorCode; + PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor; PLOCALMON_XCV pXcv;
- // Sanity checks - if (!hMonitor || !phXcv) + TRACE("LocalmonXcvOpenPort(%p, %S, %lu, %p)\n", hMonitor, pwszObject, GrantedAccess, phXcv); + + // Sanity checks + if (!pLocalmon || !phXcv) { dwErrorCode = ERROR_INVALID_PARAMETER; goto Cleanup; @@ -422,6 +433,7 @@ goto Cleanup; }
+ pXcv->pLocalmon = pLocalmon; pXcv->GrantedAccess = GrantedAccess;
if (cbObject) @@ -429,6 +441,8 @@ pXcv->pwszObject = (PWSTR)((PBYTE)pXcv + sizeof(LOCALMON_XCV)); CopyMemory(pXcv->pwszObject, pwszObject, cbObject); } + + InsertTailList(&pLocalmon->XcvHandles, &pXcv->Entry);
// Return it as the Xcv handle. *phXcv = (HANDLE)pXcv;