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

concurrency problem #15

Closed
DeoLeung opened this issue Dec 23, 2016 · 9 comments
Closed

concurrency problem #15

DeoLeung opened this issue Dec 23, 2016 · 9 comments

Comments

@DeoLeung
Copy link

I created a client

smtp = SMTP(...)

and the code of sending email

async def send_mails(smtp):
  async with smtp:
    await smtp.ehlo()
    await smtp.login()
    await smtp.send_message()

if it's called concurrently I will get errors:

aiosmtplib.errors.SMTPResponseException: (250, 'PIPELINING\nSTARTTLS\nAUTH=LOGIN\n8BITMIME')
aiosmtplib.errors.SMTPHeloError: (221, 'smtp.qq.com\nSIZE 73400320\nAUTH LOGIN PLAIN\nMAILCOMPRESS\nBye')
aiosmtplib.errors.SMTPResponseException: (-1, 'Malformed SMTP response: ')

Is there some form of locking implemented in a single instance?

@cole
Copy link
Owner

cole commented Dec 23, 2016

Yeah, there's a lock on the client that should prevent that from happening. What exactly do you mean by calling it concurrently? asyncio.gather with ehlo, login, and send_message as tasks?

@DeoLeung
Copy link
Author

I have multiple call to the send_mails function,

It's called via Tornado's spawn_callback in it's write_error function, which using asyncio

not sure exactly how it called via asyncio

so far I can avoid it as README says, creating new client for each mail

@cole
Copy link
Owner

cole commented Dec 23, 2016

Cool, thanks. I'll take a look at that.

@cole
Copy link
Owner

cole commented Dec 23, 2016

I wasn't locking the connection process, that's fixed now. Please try master and see if that works for you.

Note that you are doing extra work here potentially if you have multiple calls to this - you don't need to connect and login every time you send a message, you can connect, login, then do multiple calls to send_message.

@cole
Copy link
Owner

cole commented Jan 28, 2017

Closing as I believe this is fixed now; please reopen if that's incorrect.

@cole cole closed this as completed Jan 28, 2017
@DeoLeung
Copy link
Author

DeoLeung commented Apr 9, 2017

I tried it two way:

# single client multiple send, as you suggested
def send(smtp_client):
  if not smtp_client.is_connected:
    await smtp_client.connect()
    await smtp_client.login()
  await smtp_client.send_message()

this works fine short after connect, but after idle for a while(a few minute), calling send() will result in error response, I check the response is empty b'', it will be great if I can check when I need to re-connect and login

  File "/xxx/pyproj/modeling/handlers/__init__.py", line 182, in sendmail
    error, _ = await smtp.send_message(msg)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/smtp.py", line 233, in send_message
    sender, recipients, flat_message, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/smtp.py", line 171, in sendmail
    raise exc
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/smtp.py", line 159, in sendmail
    await self.mail(sender, options=mail_options, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/esmtp.py", line 217, in mail
    b'MAIL', from_string, *options_bytes, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/connection.py", line 195, in execute_command
    *args, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/protocol.py", line 194, in execute_command
    response = await self.read_response(timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/protocol.py", line 153, in read_response
    'Malformed SMTP response: {}'.format(full_message))
aiosmtplib.errors.SMTPResponseException: (-1, 'Malformed SMTP response: ')

the other way, as my origin post, will have Connection lost, I doubt that tornado spawn_callback may use a different loop

  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/esmtp.py", line 217, in mail
    b'MAIL', from_string, *options_bytes, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/connection.py", line 195, in execute_command
    *args, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/protocol.py", line 192, in execute_command
    await self.write_and_drain(command, timeout=timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/protocol.py", line 166, in write_and_drain
    await self._drain_writer(timeout)
  File "/xxx/.virtualenvs/mdt-dev/lib/python3.5/site-packages/aiosmtplib/protocol.py", line 227, in _drain_writer
    raise SMTPServerDisconnected(str(exc))
aiosmtplib.errors.SMTPServerDisconnected: Connection lost

at the moment I create new client SMTP() for each mail

@cole
Copy link
Owner

cole commented Apr 9, 2017

So in the first exception above, I think we should be raising SMTPServerDisconnected, not SMTPResponseException - that seems like a bug.

@cole cole reopened this Apr 9, 2017
@cole
Copy link
Owner

cole commented Apr 9, 2017

In the other case, aiosmtlib doesn't have any concept of keepalives or a connection pool or anything - it doesn't have a way to know that the server has disconnected until you try to send a command. is_connected just means the client thinks it has a connection. If you're entering the context manager each time, it should be disconnecting and reconnecting, but if you're not then all you can really do is handle the exception and retry. Something like:

try:
    await smtp_client.send_message(...)
except SMTPServerDisconnected:
    await smtp_client.connect()
    await smtp_client.login(...)
    await smtp_client.send_message(...)

If you just want to see if the connection is alive, you can see if smtp_client.noop() raises an error.

@DeoLeung
Copy link
Author

need to close before connect, otherwise it hangs, but the speed seems similar to create a new client each time, would update the thread later for comparison

try:
    await smtp_client.send_message(...)
except SMTPServerDisconnected:
    smtp_client.close()
    await smtp_client.connect()
    await smtp_client.login(...)
    await smtp_client.send_message(...)

@cole cole closed this as completed May 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants