Uploaded image for project: 'mod_python'
  1. mod_python
  2. MODPYTHON-107

mod_python.publisher shouldn't flush result when written.

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.7
    • Fix Version/s: 3.3.1
    • Component/s: publisher
    • Labels:
      None

      Description

      In mod_python.publisher, the result returned from a published function is returned as the content of the response after it has been converted to a string, using:

      req.write(result)

      In doing this, the second argument of req.write() is defaulting to '1', which indicates that the output should be flushed immediately.

      This fact prevents an output filter like CONTENT_LENGTH being used in conjunction with mod_python.publisher.

      This output filter will add a content length header to the response, but only if it is able to read the full content the first time the output filter is called. Because the output is flushed at this point, the output filter isn't able to do that, as it gets called twice. The first time it gets called will be with the actual content, the second time happens when the handler returns apache.OK and is effectively a null output call to force the output filter to be closed off.

      If instead the output is written as:

      req.write(result,0)

      the output will not be flushed immediately and instead will be output in one call when apache.OK is returned. There is no loss of efficiency in doing this and instead it will actually eliminate a redundant call into the output filter.

      For further details of this issue see discussion in:

      http://www.mail-archive.com/python-dev@httpd.apache.org/msg00951.html

      This makes one wander if there should be a configurable option for mod_python.psp to tell it not to flush output as well so that CONTENT_LENGTH could be used in that case as well. ???

        Activity

        Hide
        grahamd Graham Dumpleton added a comment -

        For PSP, the generated code actually explicitly says not to flush the output and so is okay in that respect. Unfortunately, the CONTENT_LENGTH output filter does not in work with PSP.

        The reason is that although each individual req.write() doesn't flush, the PSP code itself does an explicit flush after running the generated page code.

        exec code in global_scope
        req.flush()

        For it to work, the req.flush() call would have to be removed.

        Removing this shouldn't cause any issues when mod_python.psp is used as the handler, but it may or may not be noticeable where people use PSP.run() method from a distinct handler. This is because output would no longer be flushed by PSP.run(). Frankly though, I don't think anyone would notice as it will get flushed on any subsequent write or when handler returns.

        Show
        grahamd Graham Dumpleton added a comment - For PSP, the generated code actually explicitly says not to flush the output and so is okay in that respect. Unfortunately, the CONTENT_LENGTH output filter does not in work with PSP. The reason is that although each individual req.write() doesn't flush, the PSP code itself does an explicit flush after running the generated page code. exec code in global_scope req.flush() For it to work, the req.flush() call would have to be removed. Removing this shouldn't cause any issues when mod_python.psp is used as the handler, but it may or may not be noticeable where people use PSP.run() method from a distinct handler. This is because output would no longer be flushed by PSP.run(). Frankly though, I don't think anyone would notice as it will get flushed on any subsequent write or when handler returns.
        Hide
        jgallacher Jim Gallacher added a comment -

        Ooops. Resolved the wrong issue.

        Show
        jgallacher Jim Gallacher added a comment - Ooops. Resolved the wrong issue.
        Hide
        grahamd Graham Dumpleton added a comment -

        Attached "MP107_20060308_grahamd_1.diff" containing proposed changes. Feedback appreciated on this one.

        Changes make it so that neither mod_python.publisher or mod_python.psp will explicitly flush output. This does not mean that the output will not be sent, as the output will be sent automatically when the handler returns back to Apache at end of content/response handler phase. The reason for avoiding the flush is merely to avoiding creating an extra empty bucket in the output bucket brigade as that prevents use of certain output filters that expect all output to be in one bucket.

        Show
        grahamd Graham Dumpleton added a comment - Attached "MP107_20060308_grahamd_1.diff" containing proposed changes. Feedback appreciated on this one. Changes make it so that neither mod_python.publisher or mod_python.psp will explicitly flush output. This does not mean that the output will not be sent, as the output will be sent automatically when the handler returns back to Apache at end of content/response handler phase. The reason for avoiding the flush is merely to avoiding creating an extra empty bucket in the output bucket brigade as that prevents use of certain output filters that expect all output to be in one bucket.
        Hide
        jgallacher Jim Gallacher added a comment -

        Patch looks reasonable. I don't see any problem with PSP.run() not immediately flushing its output as the user can still call run with flush=1 if they really need that behaviour.

        Show
        jgallacher Jim Gallacher added a comment - Patch looks reasonable. I don't see any problem with PSP.run() not immediately flushing its output as the user can still call run with flush=1 if they really need that behaviour.
        Hide
        grahamd Graham Dumpleton added a comment -

        Fixed in:

        http://svn.apache.org/viewcvs?rev=384736&view=rev

        Forgot to put reference in commit message.

        Show
        grahamd Graham Dumpleton added a comment - Fixed in: http://svn.apache.org/viewcvs?rev=384736&view=rev Forgot to put reference in commit message.

          People

          • Assignee:
            grahamd Graham Dumpleton
            Reporter:
            grahamd Graham Dumpleton
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development