Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

STCC-219 fixed svg-to-png conversion error with xml register_namespace #154

Merged

Conversation

sandra0521
Copy link
Contributor

This error occured when multiple svg conversions where running at the same time and were modifying the global dictionary matching namespace uri and their prefixes. Locking the call to register_namespace seems to have fixed the problem.

Testing is difficult as this error appeared randomly and cannot be reproduced on command. It appeared most often when converting projects with a large number of images.
How I did test: converting about 200 projects where I knew those errors occured multiple times in the past (from testing and using run_statistics) and this error did not appear again.

@oiduh oiduh self-requested a review March 9, 2020 09:24
Copy link
Contributor

@oiduh oiduh left a comment

Choose a reason for hiding this comment

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

I tested your solution with some scratch projects and everything worked fine. Then I did some research to find out why this lock is necessary: Apparently the function ET.register_namespace(prefix, uri) writes to a global dictionary, accessed by all the threads, hence the race condition.
I'm not sure if I'm wrong here, but would it be possible to create a global lock in the svgtopng.py file and use it instead of the one provided as parameter? If yes, all the changes in media_converter.py, test_converter.py and test_svgtopng.py wouldn't be necessary.

@AntiDog AntiDog merged commit 39c8d8d into Catrobat:develop Mar 9, 2020
oiduh pushed a commit to oiduh/ScratchToCatrobat that referenced this pull request Mar 20, 2020
oiduh pushed a commit to oiduh/ScratchToCatrobat that referenced this pull request Mar 20, 2020
oiduh pushed a commit to oiduh/ScratchToCatrobat that referenced this pull request Mar 20, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants