ntpath contains imports inside functions

[briancurtin]

BPO	9790
Nosy	@ncoghlan, @pitrou, @briancurtin
Files	review_email.txt issue9790_fix.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/briancurtin'
closed_at = <Date 2010-09-24.13:57:55.026>
created_at = <Date 2010-09-07.13:44:01.967>
labels = ['extension-modules', 'type-bug', 'OS-windows']
title = 'ntpath contains imports inside functions'
updated_at = <Date 2010-09-24.13:57:55.024>
user = 'https://github.com/briancurtin'

bugs.python.org fields:

activity = <Date 2010-09-24.13:57:55.024>
actor = 'brian.curtin'
assignee = 'brian.curtin'
closed = True
closed_date = <Date 2010-09-24.13:57:55.026>
closer = 'brian.curtin'
components = ['Extension Modules', 'Windows']
creation = <Date 2010-09-07.13:44:01.967>
creator = 'brian.curtin'
dependencies = []
files = ['18783', '18985']
hgrepos = []
issue_num = 9790
keywords = ['patch']
message_count = 6.0
messages = ['115761', '117232', '117247', '117283', '117284', '117285']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'pitrou', 'brian.curtin']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9790'
versions = ['Python 3.2']

[briancurtin]

As pointed out by Nick Coghlan on python-dev, ntpath.samefile and ntpath.sameopenfile are vulnerable to deadlock because they contain imports.

[briancurtin]

Committed to py3k in r84988 using Nick's second suggestion.

[briancurtin]

Nick noticed another issue with this, and it actually won't work on pre-Vista versions since we load GetFinalPathNameByHandle at runtime, which is why we had that NotImplementedError.

Attaching a patch which should handle this. We only try the import on Windows 6.0 or above now, and otherwise (XP, other OS'es) use the fallback method.

[ncoghlan]

Yeah, I realised shortly after I sent my email in response to the checkin that the original version was covering more cases in that first try-block.

I'll defer to you when it comes to exactly which case that NotImplementedError was covering - patch looks good given the explanation.

[pitrou]

Patch looks ok, and it would hopefully fix build errors on the XP buildbots :)

[briancurtin]

Committed in r84992. Thanks for having a look.