Ticket #286 (closed bug: done)

Opened 6 years ago

Last modified 6 years ago

[PATCH] Split encoding/charset registration so the STRINGs for their "name" field are created *after* the necessary plugins are registered

Reported by: Infinoid Owned by:
Priority: normal Milestone:
Component: none Version:
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

Parrot encoding and character set plugins are registered at runtime through a series of init functions. The structures containing these plugins contain STRING pointers to hold the name of the plugin.

Unfortunately, there's a race condition with this process, where those name STRINGs can't be created properly yet because the encoding and charset plugins haven't always been created yet.

The result of this is that Parrot_str_new_init() is called with a NULL charset pointer, while registering the first encoding plugin, when register_encoding() tries to create the "fixed_8" STRING. This is why I marked the charset argument as NULLOK (r34712) while adding the ASSERT_ARGS() stuff (TT #105).

The only reason this worked at all is because the code would then call parrot_init_encodings_2(), which goes through and fixes up the charset pointers in the STRING structures afterwards. This is fairly hackish.

When I asked about this on IRC ( http://irclog.perlgeek.de/parrot/2008-12-30#i_794425), Allison++ gave me the idea of splitting registration from labeling. So here's a patch to do that. The patch also gets rid of the parrot_init_encodings_2() hack, and removes NULLOK from Parrot_str_new_init()'s charset argument.

I would just commit it immediately, but I wanted to first post it here for a couple of days, so others can test and review it.

Attachments

split-encoding-charset-labeling-from-init.patch Download (7.6 KB) - added by Infinoid 6 years ago.
split-encoding-charset-labeling-from-init.patch v2

Change History

follow-up: ↓ 2   Changed 6 years ago by NotFound

A big +1, but I'll prefer long descriptive unambiguous names for the internal usage only functions, something like: Parrot_str_internal_register_....

Changed 6 years ago by Infinoid

split-encoding-charset-labeling-from-init.patch v2

in reply to: ↑ 1   Changed 6 years ago by Infinoid

  • status changed from new to closed
  • resolution set to done

Replying to NotFound:

A big +1, but I'll prefer long descriptive unambiguous names for the internal usage only functions, something like: Parrot_str_internal_register_....

Thanks, I've updated the patch and checked it in as r36448.

Note: See TracTickets for help on using tickets.