Something to consider: what happens when multiple sub interpreters use the syslog extension. The current implementation always calls openlog(3) at least once before calling syslog(3), see the check in syslog_syslog_impl.

Thanks that's why I got a headache which makes me close the issue but thanks to suggest the solution.

IMHO the check in syslog_syslog_impl is not necessary because it results in a call to openlog(3) with parameters that match the default behaviour of the syslog library.

Yeah, IIUC we should call openlog(3) anyway while calling syslog.syslog right?
Please let me know if I understood it wrongly.

Yeah, IIUC we should call openlog(3) anyway while calling syslog.syslog right?

Or did you intended implicit call of openlog(3) while calling syslog(3)?

🤖 New build scheduled with the buildbot fleet by@corona10 for commit952ff46 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

Failure tests from buildbot/PPC64LE Fedora were not related to this change.

Unfortunately I don't have time to review this right now. Maybe@serhiy-storchaka can have a look? He recently did some work on this module.

Yeah, IIUC we should call openlog(3) anyway while calling syslog.syslog right?
Please let me know if I understood it wrongly.

At the C level calling openlog(3) before calling syslog(3) is not necessary, althoughthe spec is not entirely clear about this. Thelinux manual page does clearly mention that openlog(3) is optional, and matches what I remember from older Unix systems.

The call to openlog in syslog_syslog_impl basically resets the default system state (use the process name for the ident and log using the LOG_USER facility). Leaving that out should therefore be harmless, but this is technically a user-visible change of behaviour:

• C code calls openlog(...) with same parameters
• Python code calls syslog.syslog

Currently the Python code resets the values set in the first step, with my proposal it would no longer do this. I'd consider this a good change, but there's bound to be someone affected by this.

This limitation seems to be new in Python 3.12: can you document this incompatible change?
Also, can you please document in the Python syslog module documentation the behavior/limitations of sub-interpreters?

Oh I missed this, I will push the documentation ASAP.

@vstinner Updated! PTAL

@ericsnowcurrently
Would you like to take a look at the documentation update?
I am not sure it will be natural for native speakers :)

When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

I have made the requested changes; please review again

Thanks for making the requested changes!

@ericsnowcurrently,@vstinner: please review the changes made to this pull request.

@ronaldoussoren:

Why can syslog.syslog only be used when the main interpreter has called syslog.openlog? That's a restriction that is not present in the underlying library.

The limitation comes from Python which stores a Python object in a global variable:

/*  only one instance, only one syslog, so globals should be ok  */static PyObject *S_ident_o = NULL;                      /*  identifier, held by openlog()  */static char S_log_open = 0;

We must hold a strong reference because we pass a pointer into the internal UTF-8 encoded flavor of the Unicode string, PyUnicode_AsUTF8():

    /* At this point, ident should be INCREF()ed.  openlog(3) does not     * make a copy, and syslog(3) later uses it.  We can't garbagecollect it.     * If NULL, just let openlog figure it out (probably using C argv[0]).     */    if (ident) {        ident_str = PyUnicode_AsUTF8(ident);        if (ident_str == NULL) {            Py_DECREF(ident);            return NULL;        }    }

I understand that openlog() doesn't copy the ident string. Maybe this assumption is wrong and the whole change is not needed. I don't know. I don't want to check the implementation of openlog() on all supported platforms, and run a real test.

S_ident_o global variable was added in 1998 by commitae94cf2. Previously, it was astatic PyObject *ident_o = NULL; in openlog() added in 1995 by commitc1822a4.

Ok, I'm curious and I looked into the GNU glibc:https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/syslog.c;h=f67d4b58a448cabdf63f629f72a9df6e009286cf;hb=HEAD#l295

The ident string is not copied, only the pointer is copied (LogTag = ident;). So if the pointer becomes a dangling pointer, syslog() will just crash when reading ident (LogTag)

One problem with sub-interpreters is that Python objects should not travel from one interpreter to another. Butstatic PyObject *S_ident_o can become a memory copy of the UTF-8 encoded string instead, so we don't pass objects anymore.

For closelog(), I proposed earlier a counter increased by syslog.openlog() and decreased by syslog.closelog(). The C function closelog() would only be called when the counter reach zero. One problem is if two interepreters use different identifiers, the latest call to syslog.openlog() wins: it overrides the previous ident (and log level).

os.chdir() has a similar problem: it's a process-wide parameter affecting all threads and all interpreters. But it's a simpler problem. The current working directory is stored in the kernel, not in Python, and no resource have to be released explicitly at exit :-)

Agree, keeping a copy of ident in the raw dynamically allocated array will resolve the issue.

But I do not think we need a reference counter. Just call closelog() and clear the saved ident string. If your program callssyslog.closelog() multiple times, it perhaps has a trouble. But at least this will not cause a crash.

What about keeping a static buffer and copying the str object's C string into it, then giving the buffer toopenlog()? Then there's no deallocation to fuss with, we can dropS_log_open and bothsyslog.syslog() andsyslog.closelog() become simpler. I'm pretty sure the only downside is that we would introduce a hard-limit on the length of the identifier.

#defineMAX_IDENT 256staticcharS_ident[MAX_IDENT+1];...staticPyObject*syslog_openlog_impl(...){    ....constchar*ident_str=NULL;if (ident!=NULL) {Py_ssize_tsize=PyUnicode_GET_LENGTH(ident);if (size>MAX_IDENT) {PyErr_SetString(...);returnNULL;        }elseif (size>0) {strncpy(S_ident,PyUnicode_AsUTF8(ident),size);ident_str=S_ident;        }    }openlog(ident_str,logopt,facility);Py_RETURN_NONE;}staticPyObject*syslog_syslog_impl(...){if (PySys_Audit("syslog.syslog","is",priority,message)<0) {        ...    }#ifdef__APPLE__// gh-98178: On macOS, libc syslog() is not thread-safesyslog(priority,"%s",message);#elsePy_BEGIN_ALLOW_THREADS;syslog(priority,"%s",message);Py_END_ALLOW_THREADS;#endifPy_RETURN_NONE;}staticPyObject*syslog_closelog_impl(PyObject*module){    ...closelog();Py_RETURN_NONE;}

There would still be a race under per-interpreter GIL, though, if subinterpreters are allowed to callsyslog.openlog() and we don't have a process-global lock for the module.

@ronaldoussoren@vstinner@ericsnowcurrently

As the first step for the subinterpreter project, I would like to suggest maintaining restrictions for subinterpreter as the current PR.
If there are requests which require allowing all of the features from subinterpreter side, let's discuss the way to support it.
Eric's suggestion might be a good way to solve it but let's delay the decision for this moment.