987923 - GetTempDir on OS X can be called without necessary autorelease pool in place

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Josh Aas]

Attachment: Fix v1.0

From ipc/chromium/src/base/file_util_mac.mm:

bool GetTempDir(FilePath* path) {
  NSString* tmp = NSTemporaryDirectory();
  if (tmp == nil)
    return false;
  *path = FilePath([tmp fileSystemRepresentation]);
  return true;
}

The way this method is written, it depends on there being an autorelease pool in place. The assumption was probably made that this function will only be called with a event loop autorelease pool in place. There is no reason there has to be an event loop autorelease pool in place though - we're currently running into problems using this method where there is none. Best to put a local autorelease pool in place.

Comment 1

[Josh Aas]

Comment on attachment 8396631 [details] [diff] [review]
Fix v1.0

Review of attachment 8396631 [details] [diff] [review]:
-----------------------------------------------------------------

Requesting review from smichaud as he's familiar with autorelease pools.

Comment 2

[Josh Aas]

Ben Turner would like to have autorelease pools in place for all events on non-main threads. Patch coming up.

Comment 3

[Josh Aas]

Attachment: FIx v2.0

Ben Turner wanted me to also add autorelease pools to the non-main-thread event loop.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8396656 [details] [diff] [review]
FIx v2.0

There is some small chance draining the autorelease pool in the message pump will cause trouble.  But let's land this and see what happens.

Comment 5

[Josh Aas]

(In reply to Steven Michaud from comment #4)

> There is some small chance draining the autorelease pool in the message pump
> will cause trouble.  But let's land this and see what happens.

That's why I didn't add it initially but all positive and negative consequences considered I'm in favor of it.

Comment 6

[Josh Aas]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7b41d26e66

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8a7b41d26e66