#7 examine and select commands don't flush untagged responses

[Pilou]

When a select command follows an examine command:

• either a deadlock occurs
• or an error about readonly status occurs:
  • mailbox status changed to READ-ONLY
  • MAILBOX is not writable

It seems that examine and select commands don't flush untagged responses.

A simple testcase is attached.

First patch adds calls to pop_untagged_responses. Other patches are related to coding style.

[Piers Lauder]

EXAMINE and SELECT commands go to all the trouble of restoring the state of the untagged responses from any previous access to the named mailbox.

Your changes nullify this code, so how about trying instead to just comment out
the "Save state of old mailbox, restore state for new..." code above, and move your code to the start of the function, and see if this also fixes the problem.

[Piers Lauder]

The code at the top of the select method is clearly wrong in the case where
the same mailbox is being re-selected after an EXAMINE, as it keeps the READ-ONLY un-tagged response, when it should remove it. I suggest adding this line before setting the mailbox name:
self._get_untagged_response('READ-ONLY')

[Piers Lauder]

and thanks for the bug report and the other patches!

[Pilou]

These modifications fix the problem encountered with attached test.py but the read-only error still occurs using the following sequence:


M.SELECT(mailbox="DIR1", callback=cb)
M.FETCH("'1:*'", "(FLAGS UID)", callback=cb)
M.EXAMINE(mailbox="DIR2", callback=cb)
M.SELECT(mailbox="DIR3", callback=cb)
.

Attached patch update the documentation.

[Piers Lauder]

Hmmm. Instead of changing the doc to match the code, I'm now in favour of changing the code to match the doc. That is, I think all the untagged responses should in fact be flushed on an EXAMINE/SELECT command, and I can't remember why I didn't do that. However, the flush ought to be done in the code in _command after any outstanding async commands have completed.

In the meantime, you should be able to make your example work, even after my intended fix, by inserting a call to M.pop_untagged_responses() before any EXAMINE or SELECT command. Can you test that that is indeed a fix? Thanks.

[Piers Lauder]

Just to clarify, the code before EXAMINE/SELECT commands should be:
for typ,dat in M.pop_untagged_responses(): pass
for imaplib2.py versions <2.41

Or test the latest version of imaplib2.py.

[Piers Lauder]

Ignore the above. The callbacks in your code mean that the flush of the
untagged responses must not occur until after all pending asynchronous commands
have completed. So the correct solution should be the latest version (2.41).
Please try your test code using it. Thanks.

[Pilou]

The readonly error still occurs with 2.41. It seems that an untagged response is erroneously added at the end of the _put_response method. The lines 1570 and 1571 add the last untagged response of the examine call (for example GIOI5 OK [READ-ONLY] Select completed.\r\n) to the untagged responses used by the last select call. In _put_response, i don't think that _append_untagged could be used after the call to _request_pop.

[Piers Lauder]

Can you generate a log showing the error? call IMAP4() with debug=5, and upload the logfile. Thanks!

[Pilou]

• log.imaplib2: logs of imaplib2 using debug=5 (see lines 2063/2082)
• dump.imap: stream between the test program and dovecot

[Piers Lauder]

Ah, serious bug. I was processing the append_untagged for the command completion response after releasing the state change lock. I think I've fixed it. Please re-run your test with the attached version of imaplib2.py.

[Pilou]

Your patch fixes the problem, thanks !

In the previous commit, should not we use:

• either "for x,y in pop_untagged_responses(): pass"
• or "self.commands_lock.acquire()/release()"

instead of the statement "self.untagged_responses = []" ?

[Piers Lauder]

That's good to know! And thanks for your very helpful discussion. And you are right about protecting the untagged responses action - will fix and upload a new version.