Code

Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#11869 closed (fixed)

makemessages command hangs when there're many errors in the .po file

Reported by: jmv Owned by: nobody
Component: Internationalization Version: master
Severity: Keywords: gettext, msgmerge, popen3, i18n
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I have found a bug in the makemessages command. It deadlocks when spawned msgmerge utility spits too many errors or warnings (for example, about wrong escape sequences in the po file). In the ideal world these messages wouldn't appear, but we've used custom written po editor for too long and it didn't validate translations as strictly as msgmerge does, so we get quite a lot of warnings from the msgmerge.

makemessages.py uses the following code for spawning msgmerge.py:

                (stdin, stdout, stderr) = os.popen3('msgmerge -q "%s" "%s"' % (pofile, potfile), 't')
                msgs = stdout.read()
                errors = stderr.read()
                if errors:
                    raise CommandError("errors happened while running msgmerge\n%s" % errors)

which blocks when there're warnings or errors. I've googled only one report of the makemessages deadlock, so this doesn't happen often, but when it happens, it's a real PITA.

It's also very hard to repeat - sometimes it doesn't hang even if there're many errors, sometimes there're fewer errors but it hangs anyway. I believe I have one .po file which can be used to reproduce the bug, which I can share via private communication channel (for example, email, since I cannot share here where anyone would be able to see it).

In the meantime, I'll clone makemessages command in the code of my application and change the relevant code with the non-blocking version.
Eugene

Attachments (1)

11869-1.diff (3.7 KB) - added by ramiro 4 years ago.
Patch that fixes this issue

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by soulburner

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

ran into the same issue (repeatable, just had a file with a lot of error(warning because i had a lot of strings with unnamed arguments)), from python sub-process docs
"
Warning

This will deadlock if the child process generates enough output to a stdout or stderr pipe such that it blocks waiting for the OS pipe buffer to accept more data. Use communicate() to avoid that.
"
so my fix was this

from subprocess import Popen, PIPE
p = Popen(cmd, shell=True,stdin=PIPE, stdout=PIPE, stderr=PIPE)
(msgs, errors) = p.communicate()

comment:2 Changed 4 years ago by jacob

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by ramiro

Patch that fixes this issue

comment:3 Changed 4 years ago by ramiro

  • Has patch set
  • Version changed from 1.1 to SVN

I've implemented (and tested under Linux and Windows with .po files that generate 250+ msgmerge warnings) the change suggested by soulburner and the subprocess module documentation (particularly with the use of the close_fds argument argument in the Popen object).

This patch also fixes #12783 (using universal_newlines=True argument in the Popen object construction).

Feel free to change the _popen3 name I chose for the helper function.

comment:4 Changed 4 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

(In [12383]) Fixed #11869 - Prevent deadlocks when calling gettext utilities on Windows. Also fixes #12783. Thanks Ramiro Morales and soulburner.

comment:2 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.