diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index 304dc574d8..be67c9e5a2 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -18,7 +18,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.56 2010/04/01 20:12:22 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.57 2010/04/16 09:51:49 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -117,7 +117,7 @@ HANDLE syslogPipe[2] = {0, 0}; #ifdef WIN32 static HANDLE threadHandle = 0; -static CRITICAL_SECTION sysfileSection; +static CRITICAL_SECTION sysloggerSection; #endif /* @@ -268,7 +268,8 @@ SysLoggerMain(int argc, char *argv[]) #ifdef WIN32 /* Fire up separate data transfer thread */ - InitializeCriticalSection(&sysfileSection); + InitializeCriticalSection(&sysloggerSection); + EnterCriticalSection(&sysloggerSection); threadHandle = (HANDLE) _beginthreadex(NULL, 0, pipeThread, NULL, 0, NULL); if (threadHandle == 0) @@ -423,8 +424,16 @@ SysLoggerMain(int argc, char *argv[]) * On Windows we leave it to a separate thread to transfer data and * detect pipe EOF. The main thread just wakes up once a second to * check for SIGHUP and rotation conditions. + * + * Server code isn't generally thread-safe, so we ensure that only + * one of the threads is active at a time by entering the critical + * section whenever we're not sleeping. */ + LeaveCriticalSection(&sysloggerSection); + pg_usleep(1000000L); + + EnterCriticalSection(&sysloggerSection); #endif /* WIN32 */ if (pipe_eof_seen) @@ -911,17 +920,9 @@ write_syslogger_file(const char *buffer, int count, int destination) if (destination == LOG_DESTINATION_CSVLOG && csvlogFile == NULL) open_csvlogfile(); -#ifdef WIN32 - EnterCriticalSection(&sysfileSection); -#endif - logfile = destination == LOG_DESTINATION_CSVLOG ? csvlogFile : syslogFile; rc = fwrite(buffer, 1, count, logfile); -#ifdef WIN32 - LeaveCriticalSection(&sysfileSection); -#endif - /* can't use ereport here because of possible recursion */ if (rc != count) write_stderr("could not write to log file: %s\n", strerror(errno)); @@ -945,11 +946,21 @@ pipeThread(void *arg) for (;;) { DWORD bytesRead; + BOOL result; - if (!ReadFile(syslogPipe[0], - logbuffer + bytes_in_logbuffer, - sizeof(logbuffer) - bytes_in_logbuffer, - &bytesRead, 0)) + result = ReadFile(syslogPipe[0], + logbuffer + bytes_in_logbuffer, + sizeof(logbuffer) - bytes_in_logbuffer, + &bytesRead, 0); + + /* + * Enter critical section before doing anything that might touch + * global state shared by the main thread. Anything that uses + * palloc()/pfree() in particular are not safe outside the critical + * section. + */ + EnterCriticalSection(&sysloggerSection); + if (!result) { DWORD error = GetLastError(); @@ -966,6 +977,7 @@ pipeThread(void *arg) bytes_in_logbuffer += bytesRead; process_pipe_input(logbuffer, &bytes_in_logbuffer); } + LeaveCriticalSection(&sysloggerSection); } /* We exit the above loop only upon detecting pipe EOF */ @@ -974,6 +986,7 @@ pipeThread(void *arg) /* if there's any data left then force it out now */ flush_pipe_input(logbuffer, &bytes_in_logbuffer); + LeaveCriticalSection(&sysloggerSection); _endthread(); return 0; } @@ -1097,18 +1110,9 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for) _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */ #endif - /* On Windows, need to interlock against data-transfer thread */ -#ifdef WIN32 - EnterCriticalSection(&sysfileSection); -#endif - fclose(syslogFile); syslogFile = fh; -#ifdef WIN32 - LeaveCriticalSection(&sysfileSection); -#endif - /* instead of pfree'ing filename, remember it for next time */ if (last_file_name != NULL) pfree(last_file_name); @@ -1164,18 +1168,9 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for) _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */ #endif - /* On Windows, need to interlock against data-transfer thread */ -#ifdef WIN32 - EnterCriticalSection(&sysfileSection); -#endif - fclose(csvlogFile); csvlogFile = fh; -#ifdef WIN32 - LeaveCriticalSection(&sysfileSection); -#endif - /* instead of pfree'ing filename, remember it for next time */ if (last_csv_file_name != NULL) pfree(last_csv_file_name);