Bugfix: Check stream when close socket #17

Merged
merged 2 commits into from Jan 10, 2014

2 participants

@bufferx

We need to check if the socket has been released.

In general, the "close_stream" function may be called repeatedly in one loop iteration.

@paolo-losi
Owner

Hi @bufferx

thank you for the pull request. the patch is fine but,
would you mind to rework the patch with a starting "if stream is not None"
and the below all three update statements leaving the stream.close() as last?

the reason to leave stream.close as last is that it could potentially throws exceptions ...

What do you think?

@bufferx

Okay
Does your mean is that like this?

    def close_stream(self):
        if self.stream is not None:
            self.status = status.CLOSED
            self.stream.close()
            self.stream = None

Or, like this?

    def close_stream(self):
        if self.stream is None:
            return

        try:
            self.stream.close()
        finally:
            self.status = status.CLOSED
            self.stream = None

this way, we can always guarantee that the final status is correct and the IOStream object is released, no matter whatever exception occurred.

I think the second option is better, and you?

@paolo-losi
Owner

I meant the first but I like the second better.

@bufferx

Hi @paolo-losi

The patch has been updated, please recheck it. Thanks.

@paolo-losi paolo-losi merged commit d84973e into paolo-losi:master Jan 10, 2014
@paolo-losi
Owner

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment