GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-99127: Allow some features of syslog to the main interpreter only by corona10 · Pull Request #99128 · python/cpython

gh-99127: Allow some features of syslog to the main interpreter only #99128

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

Merged

corona10 merged 23 commits into python:main from corona10:gh-99127

Nov 29, 2022

Uh oh!

There was an error while loading. Please reload this page.

Merged

gh-99127: Allow some features of syslog to the main interpreter only #99128

Show file tree

Hide file tree

Changes from all commits

Commits

Show all changes

23 commits

Select commit Hold shift + click to select a range

ced41bc

gh-99127: Port syslog module to use module state.

Remove logic to check the openlog status while calling syslog.syslog

corona10 Nov 5, 2022

11d9228

Do not call openlog while calling syslog

corona10 Nov 11, 2022

f40d953

Add test

corona10 Nov 11, 2022

35bbac2

nit

corona10 Nov 11, 2022

525a32b

Address code review

corona10 Nov 12, 2022

741baf0

Update NEWS.d

corona10 Nov 12, 2022

779a3d2

nit

corona10 Nov 12, 2022

557578f

Revert some logic

corona10 Nov 12, 2022

01334e8

nit

corona10 Nov 12, 2022

4ad2b8f

Address code review

corona10 Nov 14, 2022

696196d

Update Lib/test/test_syslog.py

corona10 Nov 16, 2022

5eeade5

Address code review

corona10 Nov 16, 2022

fb81c1e

Address code review

corona10 Nov 17, 2022

140366b

Update documentations

corona10 Nov 17, 2022

c961709

Address code review

corona10 Nov 17, 2022

e8dce88

Apply suggestions from code review

corona10 Nov 18, 2022

File filter

Filter by extension .c (1) .py (1) .rst (3) All 3 file types selected

Viewed files

Clear filters

Conversations

Failed to load comments.

Uh oh!

There was an error while loading. Please reload this page.

Jump to

Jump to file

Failed to load files.

Uh oh!

There was an error while loading. Please reload this page.

Diff view

Unified

Split

Hide whitespace

Diff view

Unified

Split

Hide whitespace

There are no files selected for viewing

21 changes: 21 additions & 0 deletions 21 Doc/library/syslog.rst

Show comments

View file

Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -40,6 +40,13 @@ The module defines the following functions:
		it wasn't called prior to the call to :func:`syslog`, deferring to the syslog
		implementation to call ``openlog()``.

		.. versionchanged:: 3.12
		This function is restricted in subinterpreters.
		(Only code that runs in multiple interpreters is affected and
		the restriction is not relevant for most users.)
		:func:`openlog` must be called in the main interpreter before :func:`syslog` may be used
		in a subinterpreter. Otherwise it will raise :exc:`RuntimeError`.


		.. function:: openlog([ident[, logoption[, facility]]])

Expand All		@@ -60,6 +67,13 @@ The module defines the following functions:
		In previous versions, keyword arguments were not allowed, and ident was
		required.

		.. versionchanged:: 3.12
		This function is restricted in subinterpreters.
		(Only code that runs in multiple interpreters is affected and
		the restriction is not relevant for most users.)
		This may only be called in the main interpreter.
		It will raise :exc:`RuntimeError` if called in a subinterpreter.


		.. function:: closelog()

Expand All		@@ -72,6 +86,13 @@ The module defines the following functions:

		.. audit-event:: syslog.closelog "" syslog.closelog

		.. versionchanged:: 3.12
		This function is restricted in subinterpreters.
		(Only code that runs in multiple interpreters is affected and
		the restriction is not relevant for most users.)
		This may only be called in the main interpreter.
		It will raise :exc:`RuntimeError` if called in a subinterpreter.


		.. function:: setlogmask(maskpri)

Expand Down

9 changes: 9 additions & 0 deletions 9 Doc/whatsnew/3.12.rst

Show comments

View file

Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -649,6 +649,15 @@ Changes in the Python API
		:class:`bytes` type is accepted for bytes strings.
		(Contributed by Victor Stinner in :gh:`98393`.)

		* :func:`syslog.openlog` and :func:`syslog.closelog` now fail if used in subinterpreters.
		:func:`syslog.syslog` may still be used in subinterpreters,
		but now only if :func:`syslog.openlog` has already been called in the main interpreter.
		These new restrictions do not apply to the main interpreter,
		so only a very small set of users might be affected.
		This change helps with interpreter isolation. Furthermore, :mod:`syslog` is a wrapper
		around process-global resources, which are best managed from the main interpreter.
		(Contributed by Dong-hee Na in :gh:`99127`.)


		Build Changes
		=============
Expand Down

64 changes: 64 additions & 0 deletions 64 Lib/test/test_syslog.py

Show comments

View file

Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,6 +5,7 @@
		import threading
		import time
		import unittest
		from textwrap import dedent

		# XXX(nnorwitz): This test sucks. I don't know of a platform independent way
		# to verify that the messages were really logged.
Expand Down Expand Up		@@ -78,6 +79,69 @@ def logger():
		finally:
		sys.setswitchinterval(orig_si)

		def test_subinterpreter_syslog(self):
		# syslog.syslog() is not allowed in subinterpreters, but only if
		# syslog.openlog() hasn't been called in the main interpreter yet.
		with self.subTest('before openlog()'):
		code = dedent('''
		import syslog
		caught_error = False
		try:
		syslog.syslog('foo')
		except RuntimeError:
		caught_error = True
		assert(caught_error)
		''')
		res = support.run_in_subinterp(code)
		self.assertEqual(res, 0)

		syslog.openlog()
		try:
		with self.subTest('after openlog()'):
		code = dedent('''
		import syslog
		syslog.syslog('foo')
		''')
		res = support.run_in_subinterp(code)
		self.assertEqual(res, 0)
		finally:
		syslog.closelog()

		def test_subinterpreter_openlog(self):
		try:
		code = dedent('''
		import syslog
		caught_error = False
		try:
		syslog.openlog()
		except RuntimeError:
		caught_error = True

		assert(caught_error)
		''')
		res = support.run_in_subinterp(code)
		self.assertEqual(res, 0)
		finally:
		syslog.closelog()

		def test_subinterpreter_closelog(self):
		syslog.openlog('python')
		try:
		code = dedent('''
		import syslog
		caught_error = False
		try:
		syslog.closelog()
		except RuntimeError:
		caught_error = True

		assert(caught_error)
		''')
		res = support.run_in_subinterp(code)
		self.assertEqual(res, 0)
		finally:
		syslog.closelog()


		if __name__ == "__main__":
		unittest.main()

1 change: 1 addition & 0 deletions 1 Misc/NEWS.d/next/Core and Builtins/2022-11-05-22-26-35.gh-issue-99127.Btk7ih.rst

Show comments

View file

Open in desktop

Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Allow some features of :mod:`syslog` to the main interpreter only. Patch by Dong-hee Na.

29 changes: 27 additions & 2 deletions 29 Modules/syslogmodule.c

Show comments

View file

Open in desktop

Original file line number	Diff line number	Diff line change
Expand Up		@@ -61,10 +61,16 @@ module syslog

		#include "clinic/syslogmodule.c.h"

		/* only one instance, only one syslog, so globals should be ok */
		static PyObject S_ident_o = NULL; / identifier, held by openlog() */
		/* only one instance, only one syslog, so globals should be ok,
		* these fields are writable from the main interpreter only. */
		static PyObject *S_ident_o = NULL; // identifier, held by openlog()
		static char S_log_open = 0;

		static inline int
		is_main_interpreter(void)
		{
Comment thread corona10 marked this conversation as resolved. Resolved Uh oh! There was an error while loading. Please reload this page.
		return (PyInterpreterState_Get() == PyInterpreterState_Main());
		}

		static PyObject *
		syslog_get_argv(void)
Expand Down Expand Up		@@ -135,6 +141,13 @@ syslog_openlog_impl(PyObject module, PyObject ident, long logopt,
		long facility)
		/[clinic end generated code: output=5476c12829b6eb75 input=8a987a96a586eee7]/
		{
		// Since the sys.openlog changes the process level state of syslog library,
		// this operation is only allowed for the main interpreter.
		if (!is_main_interpreter()) {
		PyErr_SetString(PyExc_RuntimeError, "subinterpreter can't use syslog.openlog()");
		return NULL;
		}

		const char *ident_str = NULL;

		if (ident) {
Expand Down Expand Up		@@ -195,6 +208,11 @@ syslog_syslog_impl(PyObject *module, int group_left_1, int priority,

		/* if log is not opened, open it now */
		if (!S_log_open) {
		if (!is_main_interpreter()) {
		PyErr_SetString(PyExc_RuntimeError, "subinterpreter can't use syslog.syslog() "
		"until the syslog is opened by the main interpreter");
		return NULL;
		}
		PyObject *openlog_ret = syslog_openlog_impl(module, NULL, 0, LOG_USER);
		if (openlog_ret == NULL) {
		return NULL;
Expand Down Expand Up		@@ -230,6 +248,13 @@ static PyObject *
		syslog_closelog_impl(PyObject *module)
		/[clinic end generated code: output=97890a80a24b1b84 input=fb77a54d447acf07]/
		{
		// Since the sys.closelog changes the process level state of syslog library,
		// this operation is only allowed for the main interpreter.
		if (!is_main_interpreter()) {
		PyErr_SetString(PyExc_RuntimeError, "sunbinterpreter can't use syslog.closelog()");
		return NULL;
		}

		if (PySys_Audit("syslog.closelog", NULL) < 0) {
		return NULL;
		}
Expand Down

Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.