Sponsored Content
The Wayback Machine - https://web.archive.org/web/20211124030639/https://github.com/stackless-dev/stackless/issues/298
Skip to content
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

Stackman integration: race in "make -j" #298

Closed
akruis opened this issue Aug 4, 2021 · 3 comments
Closed

Stackman integration: race in "make -j" #298

akruis opened this issue Aug 4, 2021 · 3 comments

Comments

@akruis
Copy link

@akruis akruis commented Aug 4, 2021

There is a race condition in the build process, if you compile Stackless using make -j or variants.
It is caused by the rule

$(SLP_STACKMAN_OBJS): $(SLP_STACKMAN_LIB)
	cp $(SLP_STACKMAN_LIB) Stackless/platf/libstackman.a
	cd Stackless/platf && $(AR) x libstackman.a && rm libstackman.a

This rule runs once for every file in $(SLP_STACKMAN_OBJS). The race is between cp and rm. The fix is trivial.

@akruis akruis self-assigned this Aug 4, 2021
akruis pushed a commit that referenced this issue Aug 4, 2021
The was a race, when extracting libstackman.a.
@akruis
Copy link
Author

@akruis akruis commented Aug 4, 2021

Fixed

Loading

@akruis akruis closed this Aug 4, 2021
@kristjanvalur
Copy link
Collaborator

@kristjanvalur kristjanvalur commented Aug 5, 2021

Ah, so you are extracting the objects from the lib and adding them individually, but it happens for every object?
If so, then there is probably another race here, since you are extracting all objects in the ar x operation, potentially overriding an object already extracted in a previous operation. this could be harmful.
But why are you extracting the objects and not just linking with the lib? Is that because it would require bigger modifications to the standard build process?

I wonder if stackman should simply be a single .obj or .o file instead of an .a. We could supply both, I suppose. Although, in the case of at least 32 bit windows, we need both .c code (the SEH wrapping code) and the .asm code, and they create different objects.... I wonder if it is possible to merge two object files into one....

Loading

akruis pushed a commit that referenced this issue Aug 7, 2021
Kristján pointed me to another race, but now extracting the archive
members should be save.
@akruis
Copy link
Author

@akruis akruis commented Aug 7, 2021

Good point, you perfectly right. But now it should be OK.

But why are you extracting the objects and not just linking with the lib? Is that because it would require bigger modifications to the standard build process?

Yes, first I tried linking the lib, but I was not pleased with the result. The static libpython.a didn't contain the stackman object files and libstackman was listed in the output of python -m sysconfig like a system library.

Therefore changed the build process to simply extract all object files from libstackman. Now libpython.a contains the stackman object files and the existence of a libstackman.a is a hidden implementation detail of the build process. That's perfect.

I wonder if stackman should simply be a single .obj or .o file instead of an .a.

The archive is fine and more flexible.

Loading

akruis pushed a commit that referenced this issue Aug 7, 2021
Kristján pointed me to another race, but now extracting the archive
members should be save.

(cherry picked from commit 61e405e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants