Skip to content

Conversation

@skirpichev
Copy link
Member

@skirpichevskirpichev commented Jan 10, 2026

Now modification of the Struct() while packing trigger a RuntimeError
…y Struct()'s Calling the ``Struct.__new__()`` dunder without required argument now is deprecated. Calling the ``Struct.__init__()`` dunder method on initialized object now also is deprecated.
@skirpichev

This comment was marked as outdated.

@skirpichevskirpichev marked this pull request as ready for review January 10, 2026 07:55
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue is relatively independed from the concurrent packing issue, so I suggest fir it separately. We should not add deprecation in maintained versions, so I suggest to only leave checks that Struct was initialized, and add the deprecation in a separate PR.

prepare_s() should be rewritten so it only sets s_codes and other fields when it is successful.

{
PyObject*self;

if (PyTuple_GET_SIZE(args) !=1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see how can this be useful.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to trigger deprecation warning in a following scenario:

>>> import struct >>> classMyStruct(struct.Struct): ... def__init__(self): ... super().__init__('>h') ... >>> my_struct = MyStruct() <python-input-2>:1: DeprecationWarning: Struct().__new__() has one required argument

PS: deprecation part moved to #143659.

@serhiy-storchakaserhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jan 10, 2026
@skirpichevskirpichev marked this pull request as draft January 10, 2026 10:14
@skirpichev
Copy link
MemberAuthor

prepare_s() should be rewritten so it only sets s_codes and other fields when it is successful.

I think it's already the case. We can move a bit s_size/s_len assignments, but as far as we interested only in s_codes - I'm not sure if it's worth. There should be no failures after codes = PyMem_Malloc(...).

@skirpichevskirpichev marked this pull request as ready for review January 10, 2026 11:45
@serhiy-storchaka
Copy link
Member

Well, it does not matter. It would reduce the chance of race condition, but nobody should run __init__() and pack() concurrently, there are no reasons to do this, and we should not support this.

self.assertRaises(RuntimeError, meth, spam)
self.assertRaises(RuntimeError, S.iter_unpack, 1)
self.assertRaises(RuntimeError, S.pack, 1)
self.assertRaises(RuntimeError, S.pack_into, 1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S.pack_into(buffer, offset, v1, v2, ...)

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I think that argument processing in the struct's functions/methods should be refactored and transformed to the AC. E.g. S.pack_info could check required arguments before validation of the soself struct.

Though, this belongs to a separate issue.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AC cleanup goes to #143673

@skirpichev
Copy link
MemberAuthor

Well, it does not matter. It would reduce the chance of race condition, but nobody should run __init__() and pack() concurrently, there are no reasons to do this, and we should not support this.

I guess this belongs to the #143382 pr thread.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@serhiy-storchakaserhiy-storchaka enabled auto-merge (squash) January 11, 2026 15:47
@serhiy-storchakaserhiy-storchaka merged commit 515ae40 into python:mainJan 11, 2026
50 checks passed
@miss-islington-app
Copy link

Thanks @skirpichev for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 11, 2026
…y Struct()'s (pythonGH-143643) (cherry picked from commit 515ae40) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@miss-islington-app
Copy link

Sorry, @skirpichev and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 515ae4078dffa0b74e5e5431462c2f4fe4563ffa 3.13 

@bedevere-app
Copy link

GH-143695 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14 bugs and security fixes label Jan 11, 2026
@skirpichevskirpichev deleted the deprecate-struct-init/78724 branch January 11, 2026 18:04
@skirpichevskirpichev self-assigned this Jan 11, 2026
@bedevere-app
Copy link

GH-143714 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Jan 11, 2026
@skirpichevskirpichev removed their assignment Jan 11, 2026
serhiy-storchaka pushed a commit that referenced this pull request Jan 12, 2026
…dy Struct()'s (GH-143643) (GH-143695) (cherry picked from commit 515ae40) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Jan 12, 2026
reidenong pushed a commit to reidenong/cpython that referenced this pull request Jan 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@skirpichev@serhiy-storchaka